Skip to content

Conversation

@ghost
Copy link

@ghost ghost commented May 13, 2016

No description provided.

@coveralls
Copy link

coveralls commented May 13, 2016

Coverage Status

Coverage remained the same at 100.0% when pulling 5f07626 on calvinmikael:master into d5446b9 on blakeembrey:master.

@blakeembrey
Copy link
Owner

What do you think the value of these additions are? Seems like if this module broke under any version (barring normal unit tests, which is the reason to have some node versions) then JavaScript would be broken (since this module uses no specific node.js features other than remaining node 0.10 compatible).

@ghost
Copy link
Author

ghost commented May 13, 2016

The value of the addition is the ability to update the tests for express.js.

As noted as a requirement by @dougwilson in expressjs/express#2996

@dougwilson
Copy link

@calvinmikael to be clear, I never said you had to go and update the .travis.yml for all Express dependencies, only that we need to know they work first. This can easily be done locally by cloning repos, which is what I typically do.

@ghost
Copy link
Author

ghost commented May 13, 2016

@dougwilson That makes the job much easier. Thanks for the clarification.
@blakeembrey Feel free to decline the commit.

@blakeembrey
Copy link
Owner

blakeembrey commented May 13, 2016

@dougwilson Since this module obviously supports 6.x, what would you prefer here?

@calvinmikael I'd prefer to remove some existing versions then. I don't think there's any need to run 6 tests for the same 100 lines of code. I think 0.10 and stable + latest might be good 👍

Edit: To be clear, I believe Travis supports those two keywords so it might make it 10x easier to keep things up to date in the future 😄

@dougwilson
Copy link

@blakeembrey it's hard to say. Typically the code team will makes the evaluations and manages Node.js version support policies, so when an external contributor tries to do it, it certainly makes it harder. I would only expect a full report from @calvinmikael that details the 6.x support of all the modules Express depends on. For example, @calvinmikael may write that "Express uses array-flatten 1.1.1, which is a basic JavaScript module without dependencies. It should have no issues running on any Node.js version, but I cloned the repo and ran it's test suite on Node.js 6.1.0 to be sure".

This pull request was closed.
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.

3 participants