Problem/Motivation
Coding standards changes are allowed in minor releases but not patch releases since they introduce disruptions, and especially disruptive ones are scheduled during the beta and release candidate phases. For people checking against the core standard, backporting new rules can suddenly cause regressions in their compliance. (This came up again in a meeting with Mixologic at DrupalCon Vienna where we were discussing what we needed to do to enable automatic coding standards fixes via DrupalCI; the discussion underscored the need to be careful about not backporting rules in the future, although I forget all the details.) This is also even true of new rules that are technically a part of our standard but that core does not yet comply with.
On the other hand, there's also a risk in not backporting the cleanups, because the branches diverge in noisy ways that break cherry-picks of actual bugfixes that need to be backported.
In the past, changes weren't backported to the previous branch. With all the well-scoped, straightforward progress that's been happening on coding standards since DrupalCon, I think we sort of forgot about this.
Proposed resolution
We haven't had an 8.4.x patch release yet, so we can still avoid accidentally disrupting production stuff.
- Revert the changes to the ruleset only since 8.4.0 on 8.4.x only.
- Leave the cleanups themselves in 8.4.x, since a committer already considered them safe enough for backport.
- Leave 8.5.x as it is.
- In the future, backport cleanups at committer discretion if they are safe and the disruption of the cleanups themselves is limited, but enable the new rule only in the new minor branch.
- This mostly won't affect the coding standard compliance of backports, because the checks only run on the minor branch for the commit, not on the cherry-pick, and the 8.4.x ruleset will be pinned on the less restrictive version from 8.4.0.
Remaining tasks
Comment | File | Size | Author |
---|---|---|---|
#2 | coding-standards-ruleset-2919983-1.patch | 5.58 KB | xjm |
Comments
Comment #2
xjmHere's the patch.
Comment #3
xjmAlso tried to clarify this in the allowed changes policy:
https://www.drupal.org/node/2581501/revisions/view/10398592/10699558
Comment #4
catchThis makes sense, keep the clean-ups in both branches but don't backport the rulesets. And yes I at least have been backporting coding standards changes since it means that any patches against that section of the code base will apply against both branches, and they're so much tidier than they used to be, but I can see why we'd want to avoid backporting the rulesets especially since all 8.4 patches get tested against 8.5.x too.
Comment #6
catchCommitted/pushed to 8.4.x ready for the 8.4.1 release.