-
Notifications
You must be signed in to change notification settings - Fork 1.4k
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
[WIP]: refactor watcher #766
Conversation
77640f4
to
bcf6ca6
Compare
@jamestalmage sorry haven't had a chance yet to look at this. |
@novemberborn - bump! I'd like to push forward with this. There are a number of TODO comments in this PR with your name on it (quite literally). I'd like your feedback on how we should proceed. |
|
||
var debug = sinon.spy(); | ||
function proxyWalker(opts) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Walker?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@jamestalmage very cool! |
@@ -1,9 +1,12 @@ | |||
var fs = require('fs'); | |||
var path = require('path'); | |||
var nodePath = require('path'); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is the same as the above.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yeah - it's from combining the two codebases. nodePath
was used in watcher, because he used a path
variable inside methods. I can clean that up.
What's left for this to land? |
bcf6ca6
to
1d3efde
Compare
Assuming the rest of the AppVeyor runs pass, this should be mergeable. |
I assume you meant |
So far I've only moved some of the path processing stuff into AvaFiles.