Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Force fs to use rebar3 manager to fix error. See sync/fs#33 #67

Merged
merged 3 commits into from
Dec 20, 2016

Conversation

davejlong
Copy link
Member

Fixes #65. An unreleased version of fs forces the rebar3 manager to compile the dependency. See 5HT/fs#33

@coveralls
Copy link

Coverage Status

Coverage remained the same at 87.162% when pulling 04c8f00 on code-reloading into 184cc56 on master.

@@ -32,7 +32,7 @@ defmodule Kitto.Mixfile do
[{:cowboy, "~> 1.0.0"},
{:plug, "~> 1.3"},
{:poison, "~> 3.0"},
{:fs, "~> 2.11.0"},
{:fs, github: "synrc/fs"},
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can't release a hex package with a non-packaged dependency :-/

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you try this? 5HT/fs#33 (comment)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I did. It doesn't work. Needs the new fs updates on the master branch to be released.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Honestly with the repo not having git tags to match the released versions its quite hard to figure out which are the unreleased updates you're referring to. I prefer to downgrade to 0.9.2 than re-add a github dependency.

I've commented on the lack of git tags, see 5HT/fs#24 (comment).

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@davejlong did you delete a comment here?
I have this on my feed, strange..
selection_076

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I did. I looked further into exfswatch and it just seems to be a layer on top of fs so we'd end up with the same issues.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@davejlong right i considered it in the beginning, it'd be the same.

@zorbash
Copy link
Member

zorbash commented Dec 16, 2016

The best thing to do now is to raise some awareness on 5HT/fs#24 and wait. If there's no response we should consider pushing a package from our fork.

@davejlong
Copy link
Member Author

I started playing with writing a fairly simple wrapper for the fswatch library. If it works, setup would be easier as fswatch is a pure C implementation which works on Mac, *nix, Microsoft.

@zorbash
Copy link
Member

zorbash commented Dec 20, 2016

@davejlong Can you confirm that [email protected] works on the mac?

@zorbash
Copy link
Member

zorbash commented Dec 20, 2016

We have a confirmation that 2.12.0 fixes the issue. See: #65 (comment)

Merging this. Thanks @davejlong for the fantastic work on finding the root cause of this issue and doing whatever needed to resolve it. 👏 👏

@zorbash zorbash merged commit 981c2a4 into master Dec 20, 2016
@zorbash zorbash deleted the code-reloading branch December 20, 2016 14:29
@zorbash
Copy link
Member

zorbash commented Dec 20, 2016

@davejlong I've seen fswatch and it seems like a really promising attempt to make fs notifications cross-platform. What bothers me it's lack of packaging for Linux (see: https://github.com/emcrisostomo/fswatch#installation). No deb packages yet :-/

Also there's a change that fswatch will be supported through fs, see: 5HT/fs#17

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants