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

Fix for date match failing #13

Merged
merged 1 commit into from Oct 11, 2011
Merged

Fix for date match failing #13

merged 1 commit into from Oct 11, 2011

Conversation

rodnaph
Copy link
Contributor

@rodnaph rodnaph commented Oct 5, 2011

I'm using watch via the forever script, and that was detecting all files in my working copy as changed (even though none were - https://github.com/indexzero/forever/issues/129).

I tracked it down to watch seemingly incorrectly matching dates. Even though the dates are the same the == comparison fails. I don't know what the cause of this is, I suppose it's false because they're not the same date object, I can reproduce it with this script...

var x = new Date();
var y = new Date();

if ( x == y ) { 
    console.log( 'Equality matched!' );
}

if ( x.getTime() == y.getTime() ) { 
    console.log( 'getTime() matched!' );
}

Only the second will match, haven't tried this on any other system (i'm on osx with node 0.4.11).

@indexzero
Copy link

@mikeal Can you take a look at this when you're back from Portugal?

@indexzero
Copy link

It is blocking a new feature we implemented in forever: https://github.com/indexzero/forever/issues/129

@idflood
Copy link

idflood commented Oct 9, 2011

tanks rodnaph, your fix is working beautifully here.

@tauren
Copy link

tauren commented Oct 11, 2011

same problem. rodnaph's patch solved it. thanks!

mikeal added a commit that referenced this pull request Oct 11, 2011
Fix for date match failing
@mikeal mikeal merged commit 96ff3a0 into mikeal:master Oct 11, 2011
@indexzero
Copy link

@mikeal Thanks!

@indexzero
Copy link

@mikeal Oops! Spoke too soon. Can you bump the version for this and publish to npm so I can resolve the forever issue?

@indexzero
Copy link

@mikeal Ping. Can you please publish this fix?

@geddski
Copy link

geddski commented Nov 2, 2011

@mikeal could you please publish this to npm? Thanks!

@mikeal
Copy link
Owner

mikeal commented Nov 3, 2011

i'm pretty sure i already did.

@geddski
Copy link

geddski commented Nov 3, 2011

yep you did, my bad. Thanks!

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.

None yet

6 participants