Hello all, it’s time for the fortnightly coding standards meeting.

This meeting:
➤ Is for anyone interested in the Drupal coding standards.
➤ Is held on the #coding standards channel in Drupal Slack (see www.drupal.org/slack for information).
➤ Usually happens fortnightly on Tuesday 2100 UTC in Drupal Slack.
➤ The meeting open for 24 hours to allow for all timezones.
➤ Discussion is done in threads, which you can follow to be notified of new replies even if you don’t comment in the thread. You may also join the meeting later and participate asynchronously.
➤ Has a public agenda anyone by adding a comment to the meeting issue.
➤ A transcript will be made using drupal-meeting-parser and posted to the agenda issue. For anonymous comments, start with a :bust_in_silhouette: emoji. To take a comment or thread off the record, start with a :no_entry_sign: emoji.

Current ping list: @catch, @larowan, @longwave, @quietone
@dww, @borisson_ @longwave @Björn Brala, @Aaron McHale, @Alex Skrypnyk, @Urvashi, @Kingdutch

Current ping list: @catch, @larowan, @longwave, @quietone@dww, @borisson_ @longwave @Björn Brala (bbrala) Brala, @Aaron McHale, @Alex Skrypnyk, @urvashi_vora

quietone Let me know if you want to be added or removed from the above list.
Kingdutch I would like to be added :smile:
urvashi_vora I am glad to be in the list, please keep me added.

0️⃣ Who is here today? Comment in the thread to introduce yourself. We’ll keep the meeting open for 24 hours to allow for all timezones.

urvashi_vora Hi
larowlan :wave:
Alex Skrypnyk Hi all
catch :wave:
Kingdutch Hey everyone! :wave:
Björn Brala (bbrala) Björn :slightly_smiling_face: :wave:
dww Hi :wave:

1️⃣ Do you have suggested topics you are looking to discuss? Post in this thread and we’ll open threads for them as appropriate.

2️⃣ Action items

2️⃣.1️⃣ Approve previous minutes

quietone These these are waiting for review[#3382512]
urvashi_vora @quietone This is reviewed [#3382512]one concern, you forgot to add me as the participant of the meeting in Credit and commenting section. Everything else seems good. Verified the conversation from slack.
quietone @urvashi_vora, thanks. I have fixed that.If you think the minutes are correct. You can add a comment stating what you did to review them and then marked it fixed.
urvashi_vora Thanks @quietone, sure.
urvashi_vora Reviewed other issue as well[#3375432]

3️⃣ Our critical issue, #3365085: Update the coding standards process on the project page

quietone The proposal has removed all testing of any rule. I think that needs to be put back in.
quietone The community and the core committers should be able to test any rule change.
larowlan only comment is 'on going' should be 'ongoing' ?
quietone What are your thoughts on testing?
quietone I corrected the spelling in the proposal (edited)
Alex Skrypnyk Meta question - should we be discussing changes to issues here on Slack or in the actual issue queue on D.O.? Sorry I’m new to this process
quietone It can be either. Sometimes it is useful to chat in real time before making a comment on an issue.
quietone However, if a decision is reached in a Slack discussion it should be summarized and posted in the issue. The issue is the definitive record. (edited)
catch I think testing of a rule is part of the review process and/or enabling a rule for core. But I don't think it should block adopting a standard. The rules should follow the standard otherwise we end up with a chicken and egg situation.
quietone That makes sense, of course.  Maybe I am just being overly cautious. I wonder why the original process tied the two together.
catch Yeah I'm not sure. Coder definitely diverges from the standards in places but given we pick and mix with phpcs that seems less of a problem than it might have previously been.
quietone Unless there are objections here on the issue I'll update the process after the 24 hours of the meeting.
Björn Brala (bbrala) I agree with catch.
Jonathan1055 @quietone I saw your comment on the issue about removing testing of a rule, but I think @catch has explained it above. I presume by 'rule' you meant a Coder sniff and there may be standards that cannot be enforced via a Coder sniff, i.e not possible to program. The 'rules' (sniffs) should always be added or modified after the standards have been agreed, so I think the change that Catch made in comment 23 cover the situation OK.
Jonathan1055 Point 8 says
“needs documentation updates” tag removed

but there is no earlier step where that tag is added. It used to be in the old version step 8.1 but this has been re-written now.
Alex Skrypnyk I've commented on the issue https://www.drupal.org/project/coding_standards/issues/3365085#comment-1...
I think it is not ready still :disappointed:
Jonathan1055 I have also spotted a few other things. Will add them to the issue too.
catch @Alex Skrypnyk "There is no clear separation between the two, making this process look like a single process rather than two processes in two issue queues that should be actioned one after another (Standard first and Coder rule second)." this isn't always true, because sometimes we are enabling pre-existing coder rules that are already available, but not in core standards yet. (edited)
Alex Skrypnyk I'm being very pedantic here because we are defining a standard that defines standards :inception:It has to be super-clear for everyone to lower the bar
Alex Skrypnyk @catchmy point was not about the sequence of the rules - I do not know how that works exactly. My point was - it is not clear that this process consists of two components.
And, since "this isn't always true", it would be good to have a format where we can clearly capture these cases.
catch hmm, I see what you're getting at, so you think basically define two steps and reference one from the other?
catch Or two series of steps.
Alex Skrypnyk two series of steps maybe. From the discussion, I understood that Standards are completely separate from Coder rules when we are talking about a decision making process. I.e., a standard can be proposed-discussed-accepted without having a Coder rule created at all.
Jonathan1055 I agree with @Alex Skrypnyk and @catch - two series of steps, first the Coding Standards, and then optionally then Coder Sniffs process
catch I think I can adjust it for that, will try.
Jonathan1055 The current proposed steps are nearly there, it just needs the separation of the two parts, to make @Alex Skrypnyk's point clear.
Alex Skrypnyk @catch"because sometimes we are enabling pre-existing coder rules that are already available, but not in core standards yet. (edited)"
this is proabably another topic - but I do not understand why we are doing this. shouldn't there be a standard for every rule? i.e. Standards is a superset of coder rules
Alex Skrypnyk oh, I'm actually a guinea pig here - I know and practice coding standards and processes, but I do not know how this process works exactly in Drupal (I've only been a user of the coding standards, not a member of this group), so I'm reading everything from a POV of a newcomer.sorry if I'm asking too many stupid questions :slightly_smiling_face: (edited)
Jonathan1055
shouldn't there be a standard for every rule?

Coder can make use of third-party sniffs that are available but not yet adopted by Core standards. I think that's what Catch was meaning. (edited)
catch Yes exactly. But also coder pre-exists running rules on Drupal core so it is not a perfect 1-1, partly because this process has been broken for years.
Alex Skrypnyk Ok, so I think this now 3 parts:
  1. The Standard definition/acceptance
  2. The implementation in Core - because the standard may be accepted but nothing is done with it yet
  3. The implementation in Coder

(edited)
Alex Skrypnyk as for "missing standards" - should we create those?
Jonathan1055 I think it is still only two parts. The 1 and 2 above are the same thing.
catch I've posted an update, the core and coder steps can't be separated because there are too many interdependencies.
catch We might be able to implement a standard in core with no changes to coder, or it might be blocked on a coder change, or we might be able to implement it but also enforce it more strictly with coder.
catch If we try to break that into three steps it will be even more complicated than it already is.
catch @Alex Skrypnyk what do you mean by missing standards?
Alex Skrypnyk @catchread your update - I think it addresses all my concerns. Thank you. It is clear now what is going on.
Alex Skrypnyk this was my reply to
Coder can make use of third-party sniffs that are available but not yet adopted by Core standards

by "missing standards" I mean the rules that we have enabled for core but they would not be documented still as Standards. If there are any of those, of course.
I think that every rule should have a standard. If we are re-using some rules from other projects - we can just have a Standard that refers that (does not need a full description etc.). But there would be a registry of these standards that is available to everyone to review and contribute.
catch @Alex Skrypnyk pretty sure there are no rules enabled for core that don't have standards - we do have a lot of standards that aren't enforced yet, but there are some issues to fix those.
Alex Skrypnyk understood, thank you @catch
catch Documenting which standards match which rules might be useful but would be a fair amount of work.
Jonathan1055 I've made an update on the issue, for adding links and tidying up the language, readability, no major changes. There are two things still remaining:
  1. what to do with the "remove the 'documentation' tag" given that we do not specify where this is added. I think it might be added in step 5? I think that's the only place.
  2. there could be a 4th bullet point in the phpcs section, for when an existing phpcs sniff provided by Coder needs to be modified to cater for the new/altered standard. I think its worth showing that Coder maintainers are amenable to sniffs being enhanced if they can be made to fit newly adopted or modified standards. I've worked on several sniffs, and we always make them align with what Core requires. Also the primary maintainer is happy to make frequent incremental releases if Core needs a modified sniff.

(edited)
catch @Jonathan1055 both sounds good - you OK to make those changes?
Jonathan1055 will do
dww The new docs on the process are looking great. Thanks for all the work so far to improve them!
I just added a comment that the CS issue needs to include proposed changes to the documentation. I think that’d be worth mentioning, but I don’t want to derail anything.

6️⃣ Allow ommitting@var... #3238192: Allow omitting @var for strictly typed class properties

quietone catch left a comment here about making an issue in the Coder project, if there isn't one already (edited)
quietone Will someone complete this?
catch @quietone are we ready to mark this fixed otherwise?
Kingdutch I'm confused, the issue was opened with a related sibling in Coder which changes the behaviour and was released in June of last year.
So I'm not sure what further work we're waiting for.
catch I completely missed #3123282: Do not require @var tag if a property has typehint in the issue summary.
catch So yeah nothing this just needs marking fixed and the docs updating afaict.
quietone Thanks! I can do the docs updates now.
quietone Docs are updated.
quietone I presume some just needs to confirm my copy/paste and then this can be marked fixed.
Björn Brala (bbrala) Confirmed the changes in the documentation. Marking as fixed.
Björn Brala (bbrala) hmm
Björn Brala (bbrala) credits xD
Björn Brala (bbrala) @quietone I'm a bit unsure how to do credits on this. All who actively participated in the discussion in different phases? Assuming a +1 is not good enough, although that is a real contribution to getting it fixed. That would open up abuse in the future though.
Kingdutch Can I fish for issue credit as person who opened the issue and wrote the issue summary? :face_with_hand_over_mouth:
Björn Brala (bbrala) Don't think you'd need to fish.
I'm currently handling the aftermath of an outage here so can't wade through earlier meetings and such un regards to credits
Kingdutch Oof good luck! :pray:
Björn Brala (bbrala) Worked through all the threads the last few months and fixed credits to the best of my ability. Marked as fixed.
dww Huzzah! Another fixed issue. :tada:  thanks, y’all! Progress feels really good.

7️⃣  (renegade) Issue summary template - #3387167: Add an issue template for the Coding Standards project

Jonathan1055 This is good. Made some changes on the issue, as you have seen, but do we (and @quietone) want to discuss it here or fully on the issue?
Jonathan1055 The issue can get long if we keep asking questions
quietone I am available now.
Jonathan1055 thanks. only a quick one. I saw your last update on the issue. All ok, except the change you made to final step (8)
Jonathan1055 I had "Open follow-up PHPCS rules issue(s) in the Coder queue if applicable"
Jonathan1055 but you chnaged to "Create followup for PHP standard changes"
Jonathan1055 I think it needs to be specifically about Coder sniffs/rules
Jonathan1055 No, i'm referring to the template, the child issue
quietone Yes, I changed that because the followup could be a core issue.
quietone Have you had a chance to read my recent suggestions in the other issue, #3365085: Update the coding standards process on the project page#comment-15230069 (edited)
Jonathan1055 ah, not in the last two hours. I can see more now. I will read up then come back here if needed.
Jonathan1055 Yes, good changes in comment 39, makes it clearer what to do when a sniff/rule exists or does not exist. Those changes can go into the actual issue summary.
Jonathan1055 Intersting question about whether the the issue to add/modify a standard is only closed when step 8 (sniffs chnages) are all done. I'm not sure about that.
Jonathan1055 Earlier we said that making a change to a standard is one thing. Then how it is implemented does not affect whether the agreed change stands )I'm not explaining myself very well here)
catch I think it's that the change is applied, but the issue is closed once follow-ups are created. We have this with core policy issues a lot too.
catch It does make it harder to differentiate between RTBC waiting to be closed vs RTBC needs sign-offs.
Jonathan1055 Yes. If the standard is agreed and the docs have been updated then that is the purpose of the 'standards' issue and the work on the issue is done. If there are follow-ups for Coder and/or implementing in core, and those issues have been created, then I think the 'standards change' issue can be marked fixed. Is that what you mean?
quietone I think of it this way, the coding standard issue consists of two main parts, part A is to approve/disapprove the change and Part B is to implement the change. For me, Part B includes documentation and any following ups.
Jonathan1055 I'm not sure that's what @catch and @dww were viewing it as, and we tried to simplify it to be one process of steps. Surely updating documentation has to be in your Part A, otherwise nothing has actually been done. Then the part B is what we have already in the phpcs section.
quietone I was sharing a personal view. I didn't mean to confuse the issue or to suggest a change to the process.
Jonathan1055 ah, ok sorry. That's fine :slightly_smiling_face:
quietone Having said that there is the possibility of changing 'When a php standard is approved" to be step 8.
Jonathan1055 Yes I like having a step 8 for 'When a php standard is approved, if necessary create follow-up issues for Coder sniffs and/or implementing the sniffs in Core'
Jonathan1055 and if those issues have been created, and linked, the main Coding Standards Change issue can be marked 'Fixed'

Comments

quietone created an issue. See original summary.

quietone’s picture

quietone commented
Title: Coding Standards Meeting 2023-00-12 » Coding Standards Meeting 2023-09-12

quietone credited alex.skrypnyk.

quietone credited bbrala.

quietone credited catch.

quietone credited dww.

quietone credited jonathan1055.

quietone credited Kingdutch.

quietone’s picture

quietone commented
Issue summary: View changes

quietone credited larowlan.

quietone’s picture

quietone commented
Status: Active » Needs review
urvashi_vora’s picture

urvashi_vora commented

@quietone, the replies in 7️⃣ seem to be missing, and another concern, you forgot to add me as the participant of the meeting in Credit and commenting section. Everything else seems good. Verified the conversation from slack.

quietone’s picture

quietone commented
Issue summary: View changes

Adding thread #7

quietone’s picture

quietone commented

@urvashi_vora, now that I think about it, it is not documented, but credit is not given for attendance. Instead it is for being actively involved in a discussion so the issue moves forward somehow. I think anyone who says that they have done an issue review during the meeting will get credit on the issue not the meeting. I am basing this on my experience with migration meetings and the bug smash initiative meetings. If you disagree, check with the others on the committee.

Now, adding credit for @urvashi_vora for reviewing the minutes. Thanks!

But, for some reason, I always remove the participation list from the meeting minutes. I should stop doing that.

urvashi_vora’s picture

urvashi_vora commented
Status: Needs review » Reviewed & tested by the community

Thanks for the clarification @quietone.

Also I have reviewed all the conversation from the slack. After 7️⃣ being added, everything looks fine to me.

Moving it to RTBC.

Thanks

urvashi_vora’s picture

urvashi_vora commented
Status: Reviewed & tested by the community » Fixed

All the conversations are verified and are proper, thus I think we can move this issue to 'Fixed'.

Thanks

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.