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

SemVer declares API changes, but not the minimal required platform/engine/environment/framework #444

Open
koresar opened this issue May 16, 2018 · 14 comments · May be fixed by #546
Open

SemVer declares API changes, but not the minimal required platform/engine/environment/framework #444

koresar opened this issue May 16, 2018 · 14 comments · May be fixed by #546
Labels
waiting for PR Issuse that require making a PR to be resolved

Comments

@koresar
Copy link

koresar commented May 16, 2018

Please take a look at this issue: request/request#2772 (comment)

TL;DR version:

  • API did not change, so the software was releasing only the MINOR versions.
  • The software minimal required platform (in this case node.js engine version) did change from 0.10, to 4, to 6.
  • As the result the software could not be run on the unsupported platforms (node.js v0.10, v4, v6). It crashes production servers at runtime for many people out there causing revenue loss.

General best practice in node.js community:

  • Increase the MAJOR version when your software minimal platform version increases.
    (E.g. if minimum supported node.js increases from v4 to v6 you bump from v2.86 to v3.0.)

The above mentioned issue practice:

  • Follow SemVer. Meaning: no API changes -> MINOR version upgrade.

Proposal:

  • Either mention that "bumping the minimal required platform should be considered a MAJOR release". Or
  • "bumping minimal required platform should not increase the software MAJOR version".

Please note, this is a serious issue. Please, avoid platform-specific holy wars. Let's improve SemVer even further.

@jwdonahue
Copy link
Contributor

The spec is very clear that any breaking changes, which would obviously include breaking changes in dependencies, require a major version bump. If some organizations are failing to adhere to the semver guidelines, stop using their products or treat all of their releases as breaking changes.

The world is full of stupid developers and managers who will not get the message, no matter how many times you say it. The best solution to the problem referenced in your issue link is to do everything in your power to get the idiots fired ASAP, and I don't just mean the folks who ignore the SemVer spec and NPM guidelines when they publish, I mean the morons who allowed that laps of judgement to cost everyone so much money.

Auto-updating a production system from untested sources is at best, stupid, and criminally negligent in the worst cases. No amount of documentation is going to stop the criminally negligent from doing harm. Hell, even if you get them fired, they're likely to get drunk and kill somebody on the way home.

Since NPM is fairly node specific, and this particular issue seems extremely common to node.js, perhaps someone should add some static analysis to NPM to try and prevent packages from being published that violate the rules.

I have long lamented the lack of best practices documentation on this site. I even started up the wiki at one point, but that was taken away. I would be very much in favor of adding some very prominent language to the spec that included some pointers to well written guidelines in general as well as technology specific docs, but I don't think we can entertain adding one language/tool-chain specific do/don't to the spec for every use-case without rendering it incomprehensible.

@koresar, unless you intend to provide a PR for whatever intended changes you have in mind, please close this issue at your earliest possible convenience.

@koresar
Copy link
Author

koresar commented Jul 12, 2018

I'd love to submit a PR. Please, give me few days. Thank you

@jwdonahue
Copy link
Contributor

No rush, I am just trying keep the issue list down to a manageable size with actionable items that are reasonably likely to be followed through on.

Thanks!

@koresar
Copy link
Author

koresar commented Jul 29, 2018

PR #452 submitted. I'm not quite happy with what I came up with though...

@koresar koresar linked a pull request Jan 17, 2020 that will close this issue
reedy referenced this issue in silexphp/Pimple Apr 22, 2020
@alexandrtovmach alexandrtovmach added proposal question Question about SemVer and use cases labels Jun 10, 2020
@alexandrtovmach
Copy link
Member

Thanks everyone for contributions, you're amazing 🎆 Did you find any consensus?

@alexandrtovmach alexandrtovmach added consensus seeking The discussion is not over yet waiting for PR Issuse that require making a PR to be resolved labels Jun 10, 2020
@koresar
Copy link
Author

koresar commented Jun 10, 2020

The consensus is on the linked #546 PR.

@alexandrtovmach
Copy link
Member

@koresar okay, makes sense

@alexandrtovmach alexandrtovmach removed consensus seeking The discussion is not over yet question Question about SemVer and use cases labels Jun 11, 2020
@cronvel
Copy link

cronvel commented Feb 20, 2021

I ran into this recently and some users of my lib are complaining about a engine change on a semver MINOR.

I opened a question on Stack Overflow and asked Twitter about that. Isaac, author of the Node.js semver package and founder of npm answered this:

I've always considered SemVer Major to mean "you will have to change something on your end to start using this update".

If the previous version worked on platform version X, and the new version requires platform version X+n, then that qualifies, yes.

At the moment there is no resource on that matter found in search engine except this issue.
I think the SemVer spec should address this as soon as possible, because at the moment, it tells everyone that only API changes matter.

More specifically, for Node.js, this means that one would have to check ALL engine requirement for ALL dependencies, and I can't find a npm command for that.

@ljharb
Copy link
Contributor

ljharb commented Feb 20, 2021

Required engine support is part of your api.

@cronvel for npm, you can check precisely that with npx ls-engines.

@cronvel
Copy link

cronvel commented Feb 20, 2021

@ljharb Maybe, but it's still need to be explicitly said by the doc, or at least in the FAQ.

More specifically, this section is very ambiguous to that respect:

What should I do if I update my own dependencies without changing the public API?

That would be considered compatible since it does not affect the public API. Software that explicitly depends on the same dependencies as your package should have their own dependency specifications and the author will notice any conflicts. Determining whether the change is a patch level or minor level modification depends on whether you updated your dependencies in order to fix a bug or introduce new functionality. I would usually expect additional code for the latter instance, in which case it’s obviously a minor level increment.

Since there is nothing about the engine, one may think of engine as a dependency.

@ljharb
Copy link
Contributor

ljharb commented Feb 20, 2021

An implicit dependency is part of your api, since it’s something a user has to know to use your interface. The term “API” is ambiguous, but there’s no common sense interpretation of it that excludes platform support.

@cronvel
Copy link

cronvel commented Feb 21, 2021

@ljharb Still, some dumb peoples like me may lack some common sense, and need to be explicitly told things ;)

@koresar
Copy link
Author

koresar commented Feb 26, 2021

An implicit dependency is part of your api

@ljharb that's what most people think. But could you please read the link I provided in the very beginning of this issue? request/request#2772 (comment)

That's the place which caused a lot of trouble. A lot.
Those people think different than most. They DON'T think that implicit dependency is the part of your api. They don't have the same "common sense" as you do. People are different. That's why there must be no ambiguous wording in any specification. Including SemVer.

@ljharb
Copy link
Contributor

ljharb commented Feb 26, 2021

@koresar A developer with that stance is not going to be swayed by any content of any specification. They are not interested in preventing breakage, they are interested in using new features faster.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
waiting for PR Issuse that require making a PR to be resolved
Projects
None yet
5 participants