Repository navigation
Clarifying breaking changes landing in Current or LTS #660
Description
Our current documentation in the Collaborator Guide says:
* Breaking changes should *never* land in Current or LTS except when:
* Resolving critical security issues.
* Fixing a critical bug (e.g. fixing a memory leak) requires a breaking
change.
* There is TSC consensus that the change is required.The question is:
- Is the last bullet point correct? Is it @nodejs/tsc that gets to decide this or has this been delegated to @nodejs/release via their charter? I'd argue that it's been delegated based on the bullet points at https://github.com/nodejs/TSC/blob/master/WORKING_GROUPS.md#release and the mandate in https://github.com/nodejs/release. As such, TSC cannot take back that authority without de-chartering the Release WG. Ergo, TSC consensus is not what allows a breaking change to land on an LTS or Current branch. Rather, it is Release WG consensus that permits that. Agreed?
Activity
Trott commentedon Jan 29, 2019
By the way, I think the last bullet point should just be removed. The first two bullet points are consistent with the contents of the https://github.com/nodejs/release README file, so they can stay.
doc: remove outdated COLLABORATOR_GUIDE sentence about breaking changes
Trott commentedon Jan 29, 2019
Proposed fix: nodejs/node#25780
There's likely room to add in mention that the Release WG ultimately manages the contents of the LTS and Current branches, but probably not in a bullet point here? Anyway, that can be a different commit, perhaps added by a Release WG member to make sure it is worded exactly the way they want?
mhdawson commentedon Jan 30, 2019
I'm not sure that it's a black or white situation where once delegated an area has 100% control so I think if the right answer is that the TSC needs to approve then that should be ok. ie. Release team has full autonomy except that the TSC should be consulted for breaking changes in LTS releases (or some other variant for example at least an FYI).
So forgetting technicalities etc. is the right answer that the TSC does not need to be consulted or get a heads up when a breaking change goes into an LTS release?
Trott commentedon Jan 31, 2019
Forgetting technicalities: IMO the Release team does not need to do any of that. I mean, look at issues like nodejs/node#23974 listing every change, every commit, clearly labeling semver-minor and semver-major, etc. That's more than enough "heads up" and consultation. And the Release team and TSC teams overlap. Semver majors aren't going to slip in unnoticed by the TSC. Adding this requirement will just be one more checkbox (and a low-to-zero-value one at that) for the already overly burdensome and manual Release process.
Trott commentedon Jan 31, 2019
I guess it's also worth pointing out that:
A semver-major won't land in LTS or Current without a ton of discussion in places that TSC is going to notice, like the security repo or somewhere like that. And it is all but certain that Release will ping TSC anyway to let them know.
Given the above bullet point, I guess you could also make the argument that codifying TSC-notification and/or TSC-approval doesn't change much so it's worth doing. I personally don't find that argument compelling because, as the discussions in doc: remove outdated COLLABORATOR_GUIDE sentence about breaking changes node#25780 and here demonstrate, we already have too many rules that we have confusion about and perhaps even a general lack of agreement on. More rules exacerbates the need for conversations like this one. We need fewer, simpler, and more consistent rules.
Given that bullet point, you could argue for asking Release WG to either accept a charter/mandate change or to simply impose a rule themselves (which they may already have?) saying that semver majors don't land in LTS or Current without TSC approval, since that arguably simplifies things to "TSC approval needed for unusual stuff". I'd rather we streamline our collaborator docs and everyone agree that Release WG determines what goes into releases, since that's both simpler and consistent with the current status of things. But, as evidenced by this comment, reasonable people can certainly make arguments otherwise.
mhdawson commentedon Jan 31, 2019
I don't have a strong opinion. "It doesn't change much" and "a heads up does not hurt" resonates with me but I think we should go with what the Release team and TSC as a group thinks is right.
doc: remove outdated COLLABORATOR_GUIDE sentence about breaking changes
ChALkeR commentedon Feb 5, 2019
@Trott is there still anything left to do here?
doc: remove outdated COLLABORATOR_GUIDE sentence about breaking changes