- @quietone opened merge request.
- Status changed to Needs review
10 months ago 5:04am 13 June 2024 - Status changed to RTBC
10 months ago 5:17pm 13 June 2024 - πΊπΈUnited States smustgrave
This one seems straightforward as it's adding the ignore line.
- Status changed to Needs work
10 months ago 10:16am 17 June 2024 - π¬π§United Kingdom alexpott πͺπΊπ
When I read the issue title I was like uhoh I hope I agree with the implementation of this rule by coder as some of coder's opinions are highly debatable. So I reviewed all the places where we are adding skips and I think this shows the value of the rule. There are things we would not have done in core if this rule was present - so that is great. However that means I think we should add follow-ups to address all these places where we are skipping the rule and should not be.
- Status changed to Needs review
10 months ago 12:36pm 21 June 2024 - π³πΏNew Zealand quietone
Made lots of followups and committed the suggestions made in the MR. Setting back to NR.
- πΊπΈUnited States smustgrave
Should todos be placed next to the ignore comments?
- Status changed to RTBC
9 months ago 1:39pm 15 July 2024 - πΊπΈUnited States smustgrave
Removing the follow up tag as appears @quietone did all that.
Disregard my comment in #19 seeing other phpcs ignores I don't see todos.
- Status changed to Needs work
8 months ago 9:57am 4 August 2024 - π¬π§United Kingdom alexpott πͺπΊπ
The MR has conflicts with changes in HEAD
- Status changed to Needs review
8 months ago 11:14pm 6 August 2024 - π³πΏNew Zealand quietone
Rebased and added ignore for two recent changes.
- Status changed to RTBC
8 months ago 6:37pm 7 August 2024 - Status changed to Needs review
7 months ago 6:01am 30 August 2024 - Status changed to Needs work
7 months ago 1:28pm 16 September 2024 - πΊπΈUnited States smustgrave
For the 1 suggestion from @catch in the MR.
- π¦πΊAustralia larowlan π¦πΊπ.au GMT+10
Couple of questions on the MR
- π³πΏNew Zealand quietone
I expanded the followup to include all three methods that raised questions, \Drupal\ckeditor5\Plugin\CKEditor5PluginDefinition::label, \Drupal\breakpoint\BreakpointManager::getGroupLabel, and \Drupal\breakpoint\Breakpoint::getLabel. That will allow the sniff to be enabled, and prevent further violations, and leave the existing code as is.
- πΊπΈUnited States smustgrave
Resolved most of the threads but the 3 could use some feedback on.
- π³πΏNew Zealand quietone
Rebased.
Resolved most of the threads but the 3 could use some feedback on.
Feedback on what? What are the questions that need to be answered?
- πΊπΈUnited States smustgrave
Needs manual rebase.
As mentioned there are a number of these floating around if you want to focus on one and ping that to me I'll prioritize it @quietone, so you don't have to keep rebasing a dozen of them.
- π³πΏNew Zealand quietone
I have thought about that but I also like that the changes are less when rebasing frequently.
Rebase and add more ignore lines.
- πΊπΈUnited States smustgrave
No worries just made the offer to help. But if you don't mind rebasing I'll keep reviewing.
Latest rebase seems fine.
-
larowlan β
committed b49cc195 on 11.x
Issue #3123067 by quietone, smustgrave, alexpott, catch, larowlan: Fix '...
-
larowlan β
committed b49cc195 on 11.x
- π·πΊRussia zniki.ru
It looks like code duplication was made at core/modules/migrate/tests/src/Kernel/MigrateEntityContentValidationTest.php
There is fix at π Remove FormattableMarkup from FileManagedUnitTestBase Active . Automatically closed - issue fixed for 2 weeks with no activity.