Coding Standards Meeting Tuesday 30 January 2024 21:00 UTC [#3414201]

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

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.

larowlan	:wave:fyi my username is misspelled in the ping list, I'm here but also have a foot in another meeting (edited)
quietone	Sorry, I have fixed the template I copied your name from.
catch	Here belatedly.
dww	Derek, more belatedly.

0️⃣.5️⃣ Share a highlight since our last meeting, which has been over the holiday period for most of us.

quietone

Blueberry season! And my friend made 2 lovely Blueberry Clafoutis

dww

I finally started teaching maracatu in our local community again last night, 3 years after moving here! It's a beautiful, and powerful music and dance tradition from the state of Pernambuco in the northeast of Brasil (the part closest to Africa, both geographically and culturally). Deep roots in resistance to colonialism, resilience, joy, community, honoring the elements of nature. It's an honor and privilege to be able to share it here in another place still under colonial occupation.

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

2️⃣ Action items

2️⃣.1️⃣ TBD

3️⃣ Fixed since last meeting

quietone	4 meeting minutes were closed.
quietone	Issue queue triage closed 11 issues as duplicate/outdate/won't fix about two weeks ago
quietone	1 draft blog
quietone	test of issue summary template

4️⃣.1️⃣ Step 5 #3074131: Use null coalescing operator ?? instead of a ternary operator with an isset() condition

Jonathan1055	I can see in step5 you have added "Announced on 2023-11-07 and 2024-01-02" so does that mean that the period has started again? I didn't see any problem raised on feedback.
quietone	I was just trying to supply history.
quietone	I guess the it moves to Step 7, to check with core committers.
Jonathan1055	Yes it wasn't a criticism, I just wanted to know how the feedback process went. I did not see anything on the issue. Would you like me to update the issue showinf that 5 and 6 are done?
quietone	Sure!
dww	Also tagged for "Needs committer feedback": #3405089: Refine CS process based on the experience of #3303206)

4️⃣.2️⃣ Step 8 #3295249: Allow multi-line function declarations

quietone	For this I think we still need to know what to do for core. There is the sniff Squiz.Functions.MultiLineFunctionDeclaration but many of it's sub components are disabled.
quietone	Has anyone tested?
larowlan	If we have something enforcing this, then we need to remove it. But because its 'allow' and not 'require' I don't think we'd be turning anything new on
quietone	Ah, pedantic me should have realized that about 'allow' and 'require'
quietone	I recall seeing multi-line function declaration in core. I just can't find an example
mstrelan	\Drupal\Core\Extension\DatabaseDriver::__construct
quietone	Thanks, I commented on the issue linking to that constructor
mstrelan	No worries, there are quite a few more constructors like this too (edited)
dww	Although \Drupal\Core\Extension\DatabaseDriver::__construct doesn't conform to what we're proposing the standard should be. So that (and others like it) would all need updating.
dww	Don't we still want to enforce a standard on how they're formatted if they're used? Seems we still want a sniff, but it needs to be smart enough to ignore single-line functions.
dww	#3295249: Allow multi-line function declarations#comment-15423374

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

quietone	Are we waiting on the CSS community to start updating the wiki?
larowlan	@mherchel I think you were driving this one
mherchel	Just to confirm, the next step is to update the pages on Drupal.org?
larowlan	I think so

5️⃣ Stale issue #1795750: Revise coding standards to use IETF RFC 2119 standards

quietone	won't fix?
larowlan	Feels like a big undertaking
quietone	And pfrenssen explains in this comment that it was a decision to have the current 'friendly' style. #3303206: Define a standard for adding declare(strict_types=1)#comment-15354541
quietone	The friendly style was supported in that issue.
larowlan	Yes, I agree that friendly is more approachable
catch	Yeah won't fix sounds good with that one.

6️⃣ Admin

6️⃣.1️⃣ Process update, #3405089: Refine CS process based on the experience of #3303206

quietone	I think this just needs the project page to be updated. If so, who wants to do that?
dww	Related, I just updated #3387440: Add explanatory/background information to the project page with some suggestions / questions.
dww	I'd be happy to update project page for both once we're in agreement / RTBC.
dww	I have an edit tab, although interestingly I don't seem to actually be a maintainer for that project (at least according to the sidebar block).
dww	Also updated #3405089: Refine CS process based on the experience of #3303206#comment-15423359 with some more questions.
quietone	@dww, thanks for agreeing to edit the project page. I have set this to RTBC.
dww	https://www.drupal.org/project/coding_standards/issues/3405089#comment-1... done!

6️⃣.2️⃣ Reminder to sign up for your turn at admin duties in our calendar

7️⃣ Wrap Up. That's all folks for the meeting facilitation. Keep chatting in the threads and feel free to add new ones.

8️⃣ Could we consider the Git history part of the "code" and spend some energy on improving the format of our commit messages?

dww	See https://drupal.slack.com/archives/C1BMUQ9U6/p1706612626929429 for recent example
dww	In particular:#2323715: [policy, no patch] Determine format for commit credit for individuals/organizations/customers
larowlan	There's already an established standard for this- conventional commits
dww	Absolutely.
dww	It's all spelled out in these issues.
larowlan	I'm keen to explore using semantic-release for contrib
dww	The problem is it keeps stalling out.
dww	I want to drive it forward, and I'm wondering if this group can help.
dww	The "by" line must go. We need short 1-liner summaries for each commit, just like our phpdoc comments. Everything else goes "below the fold".
larowlan	https://www.conventionalcommits.org/en/v1.0.0/
larowlan	So much automation possible with ^
larowlan	All my npm packages I use github.com/semantic-release/semantic-release - you just commit with commit format per conventionalcommits and releases/changelogs/semver is all done for you
dww	Do we consider the issue ID part of the "scope"?
larowlan	even get nice comments on the PRs in each release
dww	If there's no issue ID, hard -100 million to the proposal.
dww	Oh, I see. Refs: #123
dww	Hrm, I love having those in the 1-line summaries. I'd hate to lose that by hiding such important info below the fold.
larowlan	A scope MAY be provided after a type. A scope MUST consist of a noun describing a section of the codebase surrounded by parenthesis, e.g., fix(parser):
larowlan	so that sounds like we'd need our own version of that
larowlan	https://git-scm.com/docs/git-interpret-trailers claims to be able to read the Refs: #1243 but I've never tested that
dww	Hrm. Yeah, I hope we can keep the ref in the scope (or something), but that seems like a bit of a hack.
dww	Or maybe we just have our own convention that the description always starts with #ID: ? (edited)
dww	E.g. `fix(rest): #3402981: Filename property is not updated when a duplicate file is renamed In REST and JSON API file uploads Authored-by: kim.pepper Reviewed-by: quietone Committed-by: larowlan(maybe we don't need the last since that's the author) (edited)`
dww	(That's from the latest 11.x commit I see:) `commit 501e1ed2fb9eacb351d448f57fe33a5e3b8e200b Author: Lee Rowlands <lee.rowlands@previousnext.com.au> Date: Wed Jan 31 17:59:53 2024 +1000 Issue #3402981 by kim.pepper, larowlan, quietone: Filename property is not updated when a duplicate file is renamed In REST and JSON API file uploads`
larowlan	yeah nothing in that standard prevents using #ID in description
dww	That seems vastly cleaner than shoehorning into scope.
dww	We can use 'scope' for component, more or less.
larowlan	yes that's the intended use +1
larowlan	some tooling to help with conventional commits https://commitizen-tools.github.io/commitizen/
dww	Ideally, the issue / MR suggested commit message could format all this for us.
dww	I guess all these will become gitlab labels, but whatever, we can still bolt some tooling (even if it's a browser extension or something :grimacing: ) that makes it easy to populate the merge commit message.
larowlan	core committers have tooling for commit-checks etc, so could also be something cli worst case
dww	I doubt anyone will write something to get d.o bespoke issue UI to give us these nice new messages automagically.
dww	But a CI step to build a "commit artifact" for us is super clever.
dww	Anyway, this has been a great discussion, but we're getting slightly off topic from my original intention, which was to get a sense from the rest of the committee / interested parties if this is a cause y'all are interested in collectively taking up under our own banner, or if I should bring this traffic elsewhere. :sweat_smile:
larowlan	it does fall into a governance grey area, It feels like a TWG task but I think that's largely defunct
dww	Personally, I consider the git history part of the "code"
dww	And if our authority is to help craft / enforce standards to make for legible "code", that includes the commit messages.
dww	But I'll totally grant this is subjective and a grey area. ;)
kimb0	We can specify multiple co-authors too: `Fixes: #[number redacted] Co-Authored-By: Spam King <x@y.com> Co-Authored-By: Monty Python <x@y.com>`
kimb0	instead of in the 1st line of the commit message
dww	That's the primary change I'm advocating for.
dww	From the other thread: 90% of the problem is putting all the “by” usernames between the issue ID and title. 8% are sloppy titles. 2% is wasting 5 bytes every commit with the word “Issue” that could instead tell you the category.