-
Notifications
You must be signed in to change notification settings - Fork 3.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
Get rid of warning when printing --help #994
Conversation
@Feder1co5oave: Will this break backward compatibility with older versions of Node? What versions of Node do we even support does anyone know? (Tagging @UziTech and @matt- as well) I never thought to ask whether people used Marked while running Node servers. |
I haven't thought about this, however that `stdio` option was added in node
0.7.10, so we should be ok? I think?
|
according to the current travis.yml we are testing for 0.6 but I think we should aim for Node >= 4 P.S. This doesn't really fix #992 The problem there is that We should call |
You're right, this doesn't fix 992. Still it lets us migrate to a non-deprecated option, I would merge this and pass on to think how to solve the help problem. I say we get rid of the man page. It's not being mantained. We could use the README.md file or generate a new help message. ps. Let's consider commander |
I was also thinking about updating the dependecies. We should update travis.yml and maybe package.json with a e.g. |
Good stuff here. Couple of things for my edification and whatnot. Right now we don't use a CI tool inside GitHub - and I'm not sure @chjj is considering coming with us on that at the moment. So, is the @Feder1co5oave: Can you create a ticket for the commander proposal? Given what I've seen so far from previous conversation when @chjj was more active, there is a desire for minimal dependencies, which I tend to agree with - but am not hardcore about it. Re Node version: It does seem odd that we are 0.10 - not one of the majors. Having said that, are we supporting that far back because we can - Marked doesn't need anything greater? Are we not able to take advantage of something because of that (maybe a Node standard library, or something - not a Node guy)? |
Before updating the engine version, I would try setting up travis for marked in order to test it on various node versions and see how it works out. We should also set up travis.yml such that tests are automatically run at every commit/release. @joshbruce are you by any chance able to sign up to http://travis-ci.org and enable the |
We also should update the travis.yml #967 I don't see any reason to continue supporting node 0.x You can set up travis to run on a fork but it would be nice if @chjj could hand over the repo so @joshbruce could enable travis on all PRs |
@UziTech and @Feder1co5oave: Sorry, thought I responded to this already. No, I can't. Would love to though. I don't have access to anything behind the "Settings" tab on GitHub. If I owned the repo itself, of course, that would be different. I'm finding that to be one of the drawbacks to open source projects that aren't financially backed - you really gotta love it, or be making money with it somewhere else. Of course, the alternative is a whole other set of ethical issues. |
eae056c
to
4a2c621
Compare
I think this should be added to the 1.0.0 milestone or whatever is considered the next breaking change milestone. We will likely want to drop support for Node < v4.0.0 and this (stdio) was added in v0.7.10 |
Yes. I still advocate for a 1.0.0 release as soon as possible. To quote semver.org:
Ideally, 1.0.0 means dropping support for legacy node.js environments. |
@joshbruce What does this mean? |
@Feder1co5oave Can you push to trigger a build? |
Will do as soon as I can
|
@styfle done |
@Feder1co5oave Excellent, thanks! 👍 CI is green (minus security checks), I think we should merge this as part of 0.4.0 @UziTech @joshbruce Any objections? |
I think only node >= 0.10 is being supported right now so this shouldn't
break anything?
|
Get rid of warning when printing --help
This fix.es #992https://nodejs.org/docs/latest-v8.x/api/child_process.html#child_process_options_stdio