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, @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.
| borisson_ | :wave: |
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️⃣ Approve minutes for previous meeting(s)
| quietone | https://www.drupal.org/project/coding_standards/issues/3513862 |
| quietone | https://www.drupal.org/project/coding_standards/issues/3515394 |
| quietone | Thanks to @borisson_, these are RTBC. They need credit check and marked fixed. |
2️⃣.2️⃣ Issues to go to core committer meeting
2️⃣.3️⃣ TBD
4️⃣ RTBC issues
4️⃣.1️⃣Allow omitting @var, @param and @return tags or their descriptions if the variable name and type is enough
| quietone | This is at 4) Review by the Coding Standards Committee |
| quietone | What feedback do you have for this issue? |
| nicxvan | I've been following the testing @acbramley had been doing on the sniffs, I think it looks great |
| quietone | Yes, it is good that someone else makes some tests |
5️⃣ Add conditions to deciding if a blog post is needed (edited)
| quietone | Let's change the CS process so that the blog post is only needed for disruptive or controversial changes. |
| quietone | It takes time to do and as far as I know hasn't resulted in any feedback. |
| borisson_ | No, no feedback has come from this, I think it's a good idea to reduce the admin work needed for most changes. |
6️⃣ Discuss the standards for phpunit based tests. Use this for general discussion. There will be child threads for existing issues.
| nicxvan | I don't see it, but I feel like there should be something about return types (edited) |
| nicxvan | I get phpstan failures for missing return types on tests and it feels particularly useless to need to add void to most test methods |
| nicxvan | I think we need to update this issue to address attributes too since many annotations are moving to attributes |
| nicxvan | My first comment may not be applicable to this issue |
| quietone | @nicxvan, There may not be an issue for each thing someone wants to improve. |
| quietone | Is there a way to tell phpstan to ignore test methods? |
| nicxvan | My second point was more about this line Use annotation features of PHPUnit as much as possible. |
| nicxvan | For phpstan, I am sure there is but I can't dig in at the moment |
| nicxvan | It's likely an exclude rule for a particular pattern |
| quietone | Oh right. yes, we do need to change things to use attributes not annotations. That is just a task, though, not a policy decision. |
6️⃣.1️⃣ Remove the requirement for doxygen for test methods
| quietone | It make senses to be self documenting. |
| quietone | And yet I am concerned that this will not help developers new to the code, or even the writer of said code who is looking at it many years later. |
6️⃣.2️⃣ Secondary/additional test @group names
| quietone | Is the gist of this proposal OK with everyone? |
6️⃣.3️⃣ No documentation about @group @coversDefaultClass @covers
| quietone | This just needs someone to add proposed text for these items. |
| quietone | For @group the work in the previous issue can be used,#2297541: Secondary/additional test @group names |
6️⃣.4️⃣Define a standard for documenting data providers in PHPUnit-based tests
| quietone | Since a data provider can be used by many test methods any definition of the data structure should be in the data provider. |
| quietone | It would be helpful to agree on this. It is inconsistent in core where the contents of the data array are explained. |
6️⃣.5️⃣Define a standard for naming data providers in PHPUnit-based tests
| quietone | I think it makes it easier on the next developers if there is a pattern to the names. For myself, I have come across some that didn't use 'provider' in the name and it required more effort to know what the method was for. |
| quietone | This could be resolved by documenting the data provider as discussed in the previous thread. |
7️⃣ Convert Coding Standards to GitLab pages
| quietone | Any comments or advice? |
| quietone | Someone want to RTBC? |
| borisson_ | I am not sure if I copied titles for all the pages, but to me it's probably easiest if we commit this, so we can look at it in the browser and then we can lay it next to the other pages to see what/if we we missed something |
| borisson_ | Should we also update the policy of making changes to make a pull request? |
| quietone | That seems premature. I think we need to see them on GitLab and do initial reviews first. Make sure all the links work, for one. |
Comments
Comment #2
quietone commentedComment #3
quietone commentedIssues to discuss
#3376518: Allow omitting @var, @param and @return tags or their descriptions if the variable name and type is enough
#2057905: [policy, no patch] Discuss the standards for phpunit based tests
Comment #6
quietone commentedComment #7
quietone commentedComment #8
quietone commentedComment #9
borisson_Credit looks correct and this seems complete. RTBC.
Comment #10
quietone commented@borisson_, thank you.