ayojs/ayo

Reconsidering some semver-major changes from upstream?

addaleax opened this issue · 9 comments

Currently, there are ~100 semver-major changes pending for Node 9.0.0, see https://gist.github.com/addaleax/4528d8f16838db5aaf5d2ed879eb9475 for a full list.

I’d like to consider reverting 2 of them, because I think they wouldn’t serve Ayo.js users any good and instead are just changes for the sake of change:

  • [468110b327] - (SEMVER-MAJOR) tls: deprecate parseCertString & move to internal
  • [f6caeb9526] - (SEMVER-MAJOR) os: make EOL configurable and read only

Do you think this is reasonable? Are there other changes from the list that stick out as something we might want to revisit?

Seems ok? Do you want to keep them out for ever, or just until such time as we feel like doing bumping our semver major number?

So … assuming our release tooling is going to work like Node’s, what I’d do is revert them in master. They’d be excluded until somebody reverts the reverts ;)

sounds good, I guess my only remaining question is how to keep track of things that have been kept out? I guess we could just search the reverts at some later point, but we'd still need to remember / write that down somewhere.

I'm confused why the EOL change in particular seems problematic - are people overwriting os.EOL such that them switching from assignment to defineProperty is problematic?

@sandfox Our release tooling should be pretty good at picking up commits that we have in our branches but Node doesn’t. :)

@ljharb I don’t know for sure. It’s a minor change, sure. But it also changes something that has worked basically forever, and I think we should have a better reason to justify changing that than “this should have been different from the beginning”.

Gotcha. I'd assume it can avoid issues with someone unintentionally mutating the core os object.

Has anyone run an npm search to determine if anything would break? Change for the sake of change seems just as bad as reverts solely for the sake of avoiding change :-)

Qard commented

It's a bit safer with that change though. It makes it harder to tamper with the value.

Personally I see no reason to omit these commits, but I'm not hard opposed to it.

Has anyone run an npm search to determine if anything would break? Change for the sake of change seems just as bad as reverts solely for the sake of avoiding change :-)

Node usually already does such things.

Neither of these seem particularly detrimental to me.

Closing this since it doesn’t seem to be worth pursuing, or at least not everybody feels 100 % comfortable with this :)