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

Fixed for V0.10.0 (Also backwards compatible with 0.8 and before) #33

Closed
wants to merge 1 commit into from

Conversation

NathanaelA
Copy link

Cleaned up to be JSLinted/JSHinted/"use strict". (Sorry, this does makes tracking rev's a little harder since it is a massive patch; but it was a lot easier to follow and understand what the code was doing when everything was a bit cleaner (for me) -- I'll understand if all you want to do is pick out the fixes).

…fore)

Cleaned up to be JSLinted/JSHinted/"strict".
@NathanaelA
Copy link
Author

Please note; I ran ALL the tests; and all of them passed on my Linux Box. So I assume I got everything correctly working again -- I'm not sure how the "pipe" is used so that code might have to be changed to "once" rather than always being on the new Event.

@MikeWills
Copy link

I am not sure where the source of the problem is... but forever still hangs after I implement your changes. foreversd/forever#400. Though the error messages are no longer showing.

@NathanaelA
Copy link
Author

@MikeWills -- hmm, so what happens if you go to:
/forever/node_modules/nssockets/node_modules/lazy
npm test
Does it pass all the tests on node v.0.10.0?
If all the tests pass; it might not be a lazy issue -- it might be another module that forever is using is also not v0.10.0 compatible... (Althougth it could be lazy; the "pipe" stuff wasn't real clear its usages; so I could have introduced a bug...) However, my project which uses nssocket appears to be working properly now.

@MikeWills
Copy link

[email protected] test /usr/lib/node_modules/forever/node_modules/nssocket/node_modules/lazy
expresso

sh: 1: expresso: not found
npm ERR! Test failed. See above for more details.
npm ERR! not ok code 0

@NathanaelA
Copy link
Author

LOL -- sorry about that -- I totally forgot that I had to do a "npm install" in that folder first (to get the dev dependancies); then you can do the "npm test"...

@MikeWills
Copy link

All tests pass... so like I had the feeling, it might not be your code.

@NathanaelA
Copy link
Author

Was worth a shot -- you might try doing a search of all the node_modules under the forever for any "EventEmitters" and make sure they are being created properly. See: nodejs/node-v0.x-archive#4971

@indexzero
Copy link

+1

@pkrumins @substack please help. The people are revolting on the forever issues and this appears to be the root cause.

@NathanaelA
Copy link
Author

@indexzero -- I did a "npm install forever" and then tested forever earlier today (just because I was curious about the issue) -- ; it "appears" that just copying the new "lazy.js" from my repository into the /forever/node_modules/nssockets/node_modules/lazy folder does fix it.

@pkrumins
Copy link
Owner

I just saw this message, sorry for taking so long to respond.

@NathanaelA can you sum up what your patch does? I'd love to merge yours but it changes the whole coding style.

@gabrielf
Copy link

I took the liberty to reformat the code according to the project's coding style in this gist: https://gist.github.com/gabrielf/5221167

I don't know if it can be of any help. The tests still pass with the reformat...

@MikeWills
Copy link

I still can't get it to work, but maybe I need to shut down my forever processes first before uninstalling and reinstalling. I'll test that when I have time. My problem is that forever list and both stop commands hang.

@dexmans
Copy link

dexmans commented Mar 22, 2013

Replaced old lazy.js with the formatted version from @gabrielf, works like a charm :)

@NathanaelA
Copy link
Author

@gabrielf -- Sweet, how did you re-convert it back to the old style? I decided to just publish it as is was since I saw others were complaining about the same issue and even though the code was re-formatted to the way I do my code (fully js-linted/hinted)

@pkrumins -- The main issue is that the new Node .10 changed event emitters; so the extending them had to be changed; since they also changed the streams I also have to change the way you were linking your new eventEmitter to them by adding "on" handlers to link Original -> Created And on the "pipe" it was Created -> Original. (Since it appears that that Event needs to go the opposite way). So in the .8 branch it still ties to the "data" event of the stream, under .10 it ties to the "readable" event; but then reads it and fires a "data" event so that the rest of the code can stay unchanged. :-)

I would just grab @gabrielf re-formated version so it maintains the project coding style.

@gabrielf
Copy link

I cloned the original repo, replaced lazy.js with the new one, used IntelliJ to reformat the code to a more similar style (4 spaces) then went through it line by line to fix semicolons tripple === etc.

There are indeed some inconsistencies in the formatting of the code (before as well as after) but I guess that's better to fix in a separate commit.

@pkrumins
Copy link
Owner

I just replaced lazy.js with @gabrielf's gist and I published it on npm as v1.0.9.

@MikeWills
Copy link

Restarting my server worked. 

Sent from Mailbox for iPhone

On Fri, Mar 22, 2013 at 8:06 PM, Peteris Krumins [email protected]
wrote:

I just replaced lazy.js with @gabrielf's gist and I published it on npm as v1.0.9.

Reply to this email directly or view it on GitHub:
#33 (comment)

@0b10011
Copy link

0b10011 commented Mar 28, 2013

@pkrumins, 7bf31b8 should close this PR, correct?

@pkrumins
Copy link
Owner

@bfrohs, yep!

@pkrumins pkrumins closed this Mar 31, 2013
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.

7 participants