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

Rename Script to NodeScript to match recent node versions #4

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

dudleyf
Copy link

@dudleyf dudleyf commented Dec 14, 2011

Script was changed to NodeScript in nodejs/node-v0.x-archive@75db199

@wagenet
Copy link

wagenet commented Dec 16, 2011

+1

@jonnolen
Copy link

+1, allows it to work in node 0.6.6

@telendt
Copy link

telendt commented Jan 4, 2012

@dudleyf: if your fix really makes it work with recent version of node please increment node version in package.json as well.

@wagenet
Copy link

wagenet commented Jan 5, 2012

His fix definitely makes it work in 0.6. However, making it work in both seems ideal.

@dudleyf
Copy link
Author

dudleyf commented Jan 5, 2012

@elus Thanks, if that works in 0.4, too, then that's even better. I'm assuming that if that works in 0.4 I should leave the package.json alone?

@telendt
Copy link

telendt commented Jan 5, 2012

@dudleyf: package.json specify node version as "0.4" and because of that it's impossible to install node-jsdoc-toolkit on any other node version using npm. Because this bug report is about "matching recent node versions" it should also (except of mentioned fix) change the required node version, e.g.:
"node" : "0.4 || 0.5 || 0.6"
of maybe it's even better to specify it like that:
"node" : ">=0.4"

@dudleyf
Copy link
Author

dudleyf commented Jan 5, 2012

@elus, thanks, I updated it with "node" : ">=0.4".

@telendt
Copy link

telendt commented Jan 5, 2012

Apparently there's a fork of this project in npm registry that has all already mentioned fixes (require("vm").Script + node version change in package.json). It's called jsdoc-toolkit (@p120ph37/node-jsdoc-toolkit in npm registry is called just jsdoc):
http://search.npmjs.org/#/jsdoc-toolkit
It works great in with node >=0.4 (all tests run successfully at least).

Its registry page points to @gigafied/node-jsdoc-toolkit repository at github, which doesn't seem to include mentioned fixes. Weird.

I would be really grateful if someone could explain me how is it even possible.

@gigafied
Copy link

gigafied commented Jan 6, 2012

I needed it published as an NPM package so I published a new package under a new name. I mention the fix at to top of the README, I thought I issued a pull request, but I guess not :( Anyway, looks like this pull request fixes it so no need for it now.

@p120ph37 seems to be MIA, last commit was almost 2 months ago. @dudleyf if you want to own the NPM registry that's cool with me, since you're the one with the pull request, just let me know and I can unpublish so you can publish.

@telendt
Copy link

telendt commented Jan 6, 2012

@gigafied: I still don't get one thing. How is it possible, that the latest version of jsdoc-toolkit in npm registry is different (different number version - 0.0.2 not 0.0.1, fixed Script issue in run.js, ...) than your fork in github (which seems to have only one master branch and not tags at all).

@p120ph37
Copy link
Owner

p120ph37 commented Jan 6, 2012

The tags may be local to gigafied's working copy perhaps, and not pushed back upstream to github?

On Jan 6, 2012, at 1:24 AM, Tomasz Elendt wrote:

@gigafied: I still don't get one thing. How is it possible, that the latest version of jsdoc-toolkit in npm registry is different (different number version - 0.0.2 not 0.0.1, fixed Script issue in run.js, ...) than your fork in github (which seems to have only one master branch and not tags at all).


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

@gigafied
Copy link

gigafied commented Jan 6, 2012

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.

6 participants