Problem/Motivation

There are some coder-relevant Drupal core issues blocked/postponed by coder, ideally, having a phpcs rule/sniff to generate the patch or at least to prevent future violations that's the best. However, in reality, if a code sniff is a must-have, the time spent to get an issue done would be much longer. For a simple issue, that's unreasonable to span months or longer. (I (@jungle) have requested 3 PRs to coder 3 weeks ago, they are still pending for review, see https://github.com/pfrenssen/coder/pulls)

So I'd suggest adopting a two-steps policy.

  1. Step 1: Fixing manually, if there are contributors like to do that. And meanwhile, file an issue to coder. As long as the patch is RTBC'ed. it's good to go. Do not have to wait for the sniffer.
  2. Step 2: When the sniffer gets ready, file a new issue if the issue in step 1 is closed/fixed, bringing in the new rule.

Pros:

  1. Almost coder relevant issues are novice-level issues, but writing a sniff is not a novice-level task, it's hard for most of the newcomers. Mixed/Bundled a novice-level task with a non-novice-level task, that's not newcomer-friendly.
  2. Decoupled them, if the novice-level one could get committed as soon as possible, more or less, they would be encouraged to contribute back more. Especially, it's good to new contributors if they get involved in
  3. Get the better code committed early, and patch in step 2 would be smaller and easier to review

Cons:

  1. This is one scope, which is supposed to be, being fixed by 2 core issues, this is not acceptable by the current issue scope policy.

Proposed resolution

Remaining tasks

Needs discussion.

User interface changes

API changes

Data model changes

Release notes snippet

Comments

jungle created an issue. See original summary.

chi’s picture

chi commented

+1. Apparently Drupal Coder project needs more maintainers.

jungle’s picture

jungle
He/Him
Location Chongqing, China
commented
Issue summary: View changes

Thanks, @Chi!

Correct typo in IS.

- have request
+ have requested

andypost’s picture

andypost
he/him
Primary language Russian
commented

Missing coder rules will not prevent new issues to be fixed with bugs. So -1

jungle’s picture

jungle
He/Him
Location Chongqing, China
commented

Thanks @andypost for your comment!

#3133162: Replace the start verb Test with Tests in method comments of tests let's take this one as a real example which I filed a few hours ago. If we keep waiting for a sniffer, I have no idea when it will be landed.

But if we adopt a 2-steps policy. The big patch (~500 KB) can get in soon, and from the day the step-1 patch gets committed to when a sniffer is ready, I do not think there is a lot to fix. The step-2 patch would be smaller, less than 30KB probably. And by fixing them manually at step 1, it's easy to spot edge cases, which might be good for writing the sniffer.

chi’s picture

chi commented

Technically Coder is a third party module so that we don't know for sure when those PRs will be merged.

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
commented

For me this is big -1. Coding standards fixes only have value when we can enforce them. Otherwise we'll be redoing them and encouraging more nitpick reviews. Having an automated check that committers can include in their workflow is the best way. Doing this has resulted in less coding standards errors being added to core.

Yes fixing coder can take time but if we do then not only core benefits but all of contrib and the many custom / client projects too.

Also there are 10's of open issues against core for coding standards that work and have not been fixed. If people want to get fixes in and done they can start there.

jungle’s picture

jungle
He/Him
Location Chongqing, China
commented
Status: Active » Closed (won't fix)

Thanks for your comment! @alexpott

This won't be accepted without support from core committers, so I am closing this! Thanks all for your concerns!