jshttp / negotiator

An HTTP content negotiator for Node.js
MIT License
309 stars 33 forks source link

Ship version 1 and adopt semver? #58

Closed jcrben closed 4 years ago

jcrben commented 4 years ago

In this repo, the box in the top right says Used by: 4.4m or 4.4 million packages. That plus the fact that this has been around for years suggests to me that it's ready for a stable API. Like many, I rely on this indirectly from expressjs in various projects. It's one of the few dependencies of express which hasn't hit 1.0 per http://npm.broofa.com/?q=express.

Thoughts on blockers?

dougwilson commented 4 years ago

This module does use semver currently. We can certainly bump the major version and ship a new version as 1.0.0. What breaking API changes are you proposing we make in this 1.0 release?

dougwilson commented 4 years ago

Ah, I see you edited your text after the original email to ask a question. I'm not really sure on what you mean by what are the blockers for a 1.0? This module was inherited in the 0.x series line and then just hasn't had any breaking changes made to it's API since to necessitate a major version bump, so there just hasn't been one.

I guess the question is what changes are you looking to see in a 1.0?

dougwilson commented 4 years ago

If it were me, I'd probably go back through all the open issues and pull request to get a mental map of what they are all and why they are still open. I would guess that some are not easy answers with the current API and so the API could probably be redone to take into account the additional needs that have been brought up and then that could be a 1.0. Thoughts on that?

jcrben commented 4 years ago

My question is why not just set the version to 1.0? Do you need to make any changes to get on a stable version? It sounds like you do have some changes in mind. If you could try to list them (not immediately, but when they occur) then it would be easier for the community to help you get there.

When you are on a 0.x version line, you can break the API without breaking semver. Per https://semver.org/#spec-item-4:

Major version zero (0.y.z) is for initial development. Anything MAY change at any time. The public API SHOULD NOT be considered stable.

I also don't see a disclosure that you version based on semver in the README. I tend to grep a readme for semver when I'm thinking about using a library. There really should be a machine-readable way to communicate that but I don't know if there is.

dougwilson commented 4 years ago

My question is why not just set the version to 1.0?

This module was inherited in the 0.x series line and then just hasn't had any breaking changes made to it's API since to necessitate a major version bump, so there just hasn't been one.

Do you need to make any changes to get on a stable version?

Ideally it would just be resolving the outstanding issues in this repo and landing the outstanding PRs.

If you could try to list them (not immediately, but when they occur) then it would be easier for the community to help you get there.

AFAIK it's the currently opened issues and PRs.

I also don't see a disclosure that you version based on semver in the README.

Sure, but also, it seems that if this module was 1.x.x based on this issue, you would have actually assumed it was? I'm not really sure about that. I've actually just never encountered npm modules saying what versioning scheme they are using, as I was under the assumption that following semver was a contract of publishing to npm registry at all, right?

Anyway, I believe the initial question is answered 👍 If you believe there is an action item that should keep this issue open please let me know and I can re-open it. The only action item I really saw was to list the changes that should be made, which are already listed in the repo's issue tracker.

jcrben commented 4 years ago

Thanks for the response. I'm fine with your clarification and closing this, but just a couple thoughts:

then just hasn't had any breaking changes made to it's API since to necessitate a major version bump

I don't think you have to make breaking changes to update to a stable version. I can see how that is a bit ambiguous from the spec, but they really encourage you to hit 1.0 when it's being used in production: https://semver.org/#how-do-i-know-when-to-release-100

as I was under the assumption that following semver was a contract of publishing to npm registry at all, right?

I don't think so. It's a recommendation (see https://docs.npmjs.com/about-semantic-versioning), but they don't require it. I believe there are packages on npm which explicitly do not follow semver. The author of underscore, for example, was notably a semver skeptic: https://gist.github.com/jashkenas/cbd2b088e20279ae2c8e - which is why his packages are on this list https://github.com/boneskull/npm-non-semver

dougwilson commented 4 years ago

Sure, but your post title made it sound like you didn't think this module used semver and then later said that you were expecting the README to explicitly state it used semver somewhere. I'm not aware of modules for npm making such a statement because the npm tooling assumes semver. Someone making a list to "call out" the module that do not follow semver on npm seems to be another reinforcement that the expectation is a module on npm is using semver, no?

jcrben commented 4 years ago

Ah, I see - sorry about the wording. I suspected this used semver, but the lack of a 1.0 major version makes it a bit less meaningful since it doesn't yet have a stable API (from the machine's perspective). Plus I like to see that statement as it makes it explicit and top of mind. Ultimately, I'm a free rider on your work, and I greatly appreciate that work!

dougwilson commented 4 years ago

Ultimately I agree that we should release a 1.0 -- I just want to make sure you don't think I'm just dismissing your issue. I closed it mainly because being a "meta" issue doesn't, in itself, move anything forward, besides just saying "hey, I think this should be a 1.0" (and which it noted). Due to you bringing up the issue I've been thinking a bunch about the open issues and what should be landed and then make it a 1.0, as there are both missing features, and some of the default behavior does need to change, which would be breaking indeed. But even then, at this point even a new feature publish should probably be a 1.0 to get this to 1.0 status.

I have gotten quite a few of the other adopted modules into 1.0 over the last couple years. The "cookies" module is another notable one that needs to really just be a 1.0 as well. It's definately on my mind not to have 0.x modules and have been following npm's recommendation of just starting the module at 1.0.0 and skipping all the 0.x nonsense as well, haha