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. Alternating between Tuesday 2100 UTC and Wednesday 0900 UTC.
➤ The meeting open for 24 hours to allow for all time zones.
➤ 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.
➤ The transcript will include comments made during the 24 hours of the meeting. However, comments made after the 24 hours may not be in transcript.

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

Comments

quietone created an issue. See original summary.

bbrala credited apaderno.

bbrala credited borisson_.

bbrala credited catch.

bbrala credited larowlan.

bbrala credited mstrelan.

bbrala credited urvashi_vora.

bbrala’s picture

bbrala
Location Netherlands
commented

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 time zones.

quietone Hi.
kimb0 Hey!
mstrelan mstrelan
Björn Brala (bbrala) Hi! :smile:
urvashi_vora :wave: Hello
JEET Hello

1️⃣ What topics do you want to discuss? Post in this thread and we’ll open threads for them as appropriate.

mstrelan Maybe we can open a thread in old issues for #3455550: Rename mis-named data providers discovered in #3421418

2️⃣ Action items

quietone someone to parse the minutes for the last two meetings

2️⃣.1️⃣ Approve minutes for previous meeting(s)

larowlan #3445915: Coding Standards Meeting Tuesday 2024-05-21 2100 UTC
Björn Brala (bbrala) It is rtbc afaik? I'm not closing it :stuck_out_tongue_winking_eye:
urvashi_vora I marked it as fixed.
urvashi_vora Thanks @Björn Brala (bbrala) for the review.

4️⃣ .1️⃣ #3339746: Coding style for PHP Enumerations

larowlan This was in the blog post and would have been approved at our Jun 5 meeting, but we skipped that one. We can deal with it today - https://www.drupal.org/about/core/blog/coding-standards-proposals-for-fi...
Björn Brala (bbrala) Ok, thats good. Just post an update to the issue or here 🙂
quietone I thought I commented here on the day of the meeting?This went to the core committer meeting and there were no objections. (edited)
quietone There are no objections on the issue either and the discussion period has passed.
quietone Still to do are doc updates and a coder issue.
quietone Who wants to do those two items?

4️⃣.2️⃣ #3324368: Update CSS coding standards to include PostCSS and Drupal 10

quietone I moved one page from the google doc to the Drupal Wiki. As I recall that was the easiest page.
quietone That should be reviewed.
quietone But there is more to do.

6️⃣ Old issues (edited) 

quietone How lovely, I do like the old issues.
urvashi_vora Let's fix...

6️⃣.1️⃣ #1991932: Standard for assigning references to variables

quietone $ grep -Irn "= &" core | wc -l
332
$ grep -Irn "=&" core | wc -l
155
quietone Is this already covered in https://www.drupal.org/docs/develop/standards/php/php-coding-standards#o... binary operators (operators that come between two values), such as +, -, =, !=, ==, >, etc. should have a space before and after the operator, for readability.
quietone But is there a sniff?
mstrelan is =& a binary operator? i don't think so. i've always use $foo = &$bar treating & as a modifier to $bar, not sure that's the right terminology.
mstrelan example in https://www.php.net/manual/en/language.oop5.references.php uses that style too:$c = new A;
$d = &$c; // $c and $d are references
// ($c,$d) =
mstrelan github shows 306k results for = &$ and 178k results for =& $
catch I would say the binary coding standards kind of covers it since $a = &$b does have a space after the operator.
apaderno I guess people write $d =& $c; because they do not notice PHP does not have any =& operator. (edited)
apaderno Should the text make clear that operator does not mean any combination of operators? (edited)
quietone I updated the issue with the new template so we can continue to work through this issue.

6️⃣.2️⃣ #1561464: Introduce @was phpDoc directive to link/reference to previous/renamed functions

quietone A renamed method/function will now have a deprecation message. That should help here.
quietone This would be very useful when doing git archaeology. But, for me, that is not enough to add this extra documentation burden.
mstrelan i'd be more interested in a #[Deprecated] attribute, there is a php rfc for this too https://wiki.php.net/rfc/deprecated_attribute
quietone Agreed.
mstrelan i guess @was is meant for when the deprecated function is actually removed though
mstrelan but with tools like drupal-rector this might not be as relevant today as it was back then
quietone You are saying what I am thinking.
quietone A problem in the IS is that 'Renamed functions are hard to figure out in the API documentation."
quietone Now that deprecation messages are defined with 'use X instead' I think that is covered.
quietone Example, https://api.drupal.org/api/drupal/core%21lib%21Drupal%21Core%21Extension...…]e.php/function/ThemeHandlerInterface%3A%3ArebuildThemeData/10
mstrelan Only part missing I think is if you are looking at the D12 api doc for ThemeHandlerInterface::rebuildThemeData you're not going to find it. But I really don't like the idea of keeping around references to old code.
mstrelan But I'm not sure if that's even what the issue is about, I think point 3 of the proposed resolution is the crucial part:Make Coder Upgrade query and pull in the current list of @was references directly from api.drupal.org and use a generic review rule to warn that $before became $after.and we have that covered with rector
Björn Brala (bbrala) I think the current set of controls we have, deprecation notices, change records and static analysis should be fine. I would vote to close this as it is superseded by the evolving ecosystem by now 🙂
borisson_ I agree, I think we can close this
quietone I agree as well.
quietone I updated and closed a won't fix.

6️⃣.3️⃣ #1567920: Naming standard for abstract/base classes

quietone This is the relevant section, https://www.drupal.org/docs/develop/standards/php/object-oriented-code#n...
Björn Brala (bbrala) Although I kinda like Abstract in class names when they are abstract, the can of worms should we do this is pretty bit.The fact core is kinds inconsistent does worry me a little, I'm not sure if defining this is really worth the time and effort that it will spawn.
quietone Would it make sense to make a standard and then say that it applies only to classes created after the standard is approved?
Björn Brala (bbrala) That would also mean the definition of when to use base vs abstract. Reading the isue Seems core is kinda inconsistent in regards to that.Validating this will also be kinda hard. We can, but we will need quite a few exclusions then also if we are not to enforce this on the old classes. And I'd kinda prefer validation.I don't feel strongly about all this. Just is a little more complex than average
quietone Updated the IS. This needs more discussion.

6️⃣.4️⃣ #1539738: list() syntax

quietone There are inconsistencies in core on this in the use of the comma. Most instances use ", ," but there are some with ",,"$ grepex -r "^\s*\[.*\] = " core | awk -F: '{ print $2}' | awk '{ print $1=$1 };1' | sort -u | grep ,,
[,,
[,, $display_mode] = explode('.', $config_id);
[$entity_type_id,,
[$entity_type_id,, $uuid] = explode('
quietone I didn't find anything in our standards for this.
quietone It looks like we need to standardize the comma style. And then a new sniff.
Björn Brala (bbrala) Didn't find a sniff easily, but searching is kinda hard with the word 'list'. I would also love to see this defined and have a rule in place for it.

6️⃣.5️⃣ https://www.drupal.org/project/coding_standards/issues/782016

quietone Can someone find an example?

7️⃣ Issues in review blog post (edited) 

larowlan Only https://drupal.slack.com/archives/C02LJCF78E8/p1718744593600879
quietone This was brought at the last committer meeting and there was no objection raised about this standard.
quietone And there has been no further feedback on the issue.

6️⃣.7️⃣ via @mstrelan #2918440: Define a standard for documenting data providers in PHPUnit-based tests

quietone I have added the new template for this one.
quietone We might want to split this into two 1) for the doc bloc and 2) for naming.
Björn Brala (bbrala) For removing test from dataproviders there is actually a rector: https://github.com/rectorphp/rector-phpunit/blob/main/docs/rector_rules_...
quietone This is now two issues. One for doc bloc documentation and the other for naming.

Participants:

mstrelan, quietone, larowlan, bbrala, urvashi_vora, catch, apaderno, borisson_

bbrala’s picture

bbrala
Location Netherlands
commented
Status: Active » Needs review
larowlan’s picture

larowlan
Location 🇦🇺🏝.au GMT+10
commented
Status: Needs review » Fixed

Status: Fixed » Closed (fixed)

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