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

growl v1.10.1 has a breaking change in a patch version. #79

Open
ljharb opened this issue Aug 31, 2018 · 16 comments
Open

growl v1.10.1 has a breaking change in a patch version. #79

ljharb opened this issue Aug 31, 2018 · 16 comments

Comments

@ljharb
Copy link

ljharb commented Aug 31, 2018

v1.10.0 has no arrow functions, v1.10.1 does. This breaks on node < 4, which is a breaking change in a patch version.

ljharb added a commit to es-shims/es5-shim that referenced this issue Aug 31, 2018
jasmine-node v1.14+ depends on ~1.0.1 of jasmine-growl-reporter: mhevery/jasmine-node#433
jasmine-growl-reporter v1.0.1 depends on growl ^1.10.2: AlphaHydrae/jasmine-growl-reporter#3
growl v1.10.1+ has a breaking change: tj/node-growl#79
@deiga
Copy link
Collaborator

deiga commented Sep 3, 2018

@ljharb You are quite right, I broke the SemVer contract with that change, and I am terribly sorry for it.

@ljharb
Copy link
Author

ljharb commented Sep 3, 2018

@deiga things happen, np. Are you planning to release a patch fixing this?

@deiga
Copy link
Collaborator

deiga commented Sep 4, 2018

@ljharb I'm trying to figure out a nice way to do this.
Quick and dirty would be to just bump minor version.
A cleaner option is to revert the ES6 changes and publish a patch, but there is a big problem with that since we don't really want to support unsecure node versions.
Probably the cleanest option would be to unrelease everything up to 1.9.2 and then rerelease with a bigger minor version, but that is also the most work 😁

@ljharb
Copy link
Author

ljharb commented Sep 4, 2018

Bumping the minor woudnt help - adding arrow functions requires a major bump - ie, v2.

There’s nothing insecure about older nodes.

The easiest is to either convert the arrows to normal functions, or, to add babel.

@deiga
Copy link
Collaborator

deiga commented Sep 5, 2018

Since there are no interface contract changes a Major version bump is not warranted for possibly breaking changes. Minor versions are exactly for this kind of case.

And yes, since everything below LTS is unsupported they are all inherently insecure, since there are no security patches being made.

@ljharb
Copy link
Author

ljharb commented Sep 5, 2018

That's not how semver works - any possibly breaking changes are major, minor is only for additions. Whether you want to drop support for something or not, dropping support for it is always a breaking change. v1.10.1 thus should have been a v2, and it would be appropriate to release a patch that reverts back to v1.10.1's level of support, and then rerelease as a v2 if desired.

@deiga
Copy link
Collaborator

deiga commented Sep 6, 2018

@ljharb You are absolutely correct, I mis-remembered how the version definitions went

AlphaHydrae added a commit to AlphaHydrae/jasmine-growl-reporter that referenced this issue Sep 7, 2018
@ljharb
Copy link
Author

ljharb commented Sep 12, 2018

@deiga any update?

@deiga
Copy link
Collaborator

deiga commented Sep 12, 2018

@ljharb Not really. I'm really busy at work and haven't had the energy to focus on OSS projects

@ljharb
Copy link
Author

ljharb commented Sep 12, 2018

I’m happy to manually revert the problematic syntax and publish an update for you, if you want to delegate commit/publish rights.

@deiga
Copy link
Collaborator

deiga commented Sep 13, 2018

@ljharb Unfortunately I don't have the power to do that, only @tj can delegate rights.

@plroebuck
Copy link

What's the status of this issue?

@deiga
Copy link
Collaborator

deiga commented Nov 9, 2018

@plroebuck This is still in not progressed -status. Every time I come back to this I get a bit overwhelmed of what steps I should actually be taking and then I just do something else instead.

@ljharb
Copy link
Author

ljharb commented Sep 21, 2021

@tj any chance you'd be willing to add me to the repo/package so i can address this?

@deiga
Copy link
Collaborator

deiga commented Sep 24, 2021

@ljharb At this point, I would suggest forking the package and making it right there. TJ is not active on this and doesn't want to pass on the keys to the kingdom either

@ljharb
Copy link
Author

ljharb commented Sep 24, 2021

That won’t solve the problem, because this package is transitively depended on by many things. The only solution is to wait until you have the time/energy, or, until TJ adds someone who does.

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

No branches or pull requests

3 participants