- 🇨ðŸ‡Switzerland berdir Switzerland
Like I said in 📌 Drupal best practices and coding standards Needs work :
Coding standard improvements must be provided as merge requests now, so that we can verify it using GitlabCI.
Additionally, this is way too big as a single patch and overlaps with many other issues. This is impossible to review and needs to be split up into issues for specific changes or groups of related changes.
- First commit to issue fork.
- First commit to issue fork.
- last update
over 1 year ago PHPLint Failed - 🇮🇳India sakthi_dev
Updated Issue Summary and also created a MR 53. Please review. Still minified css error is there.
FILE: ...me/administrator/Projects/contribution/token/css/token.treetable.theme.css -------------------------------------------------------------------------------- FOUND 0 ERRORS AND 1 WARNING AFFECTING 1 LINE -------------------------------------------------------------------------------- 1 | WARNING | File appears to be minified and cannot be processed --------------------------------------------------------------------------------
- 🇷🇺Russia zniki.ru
Nikolay Shapovalov → made their first commit to this issue’s fork.
- last update
over 1 year ago 36 pass, 38 fail - last update
over 1 year ago 77 pass - 🇷🇺Russia zniki.ru
As requested by @Berdir I will create several smaller MRs, not sure if each MR should have separate issue.
First one https://git.drupalcode.org/project/token/-/merge_requests/54 is ready for review. - last update
over 1 year ago 77 pass - Merge request !56Resolve #2774071: Fix Drupal.Arrays, SlevomatCodingStandard.PHP.ShortList.LongListUsed → (Open) created by zniki.ru
- last update
over 1 year ago 77 pass - Status changed to Needs review
over 1 year ago 2:55pm 28 December 2023 - last update
over 1 year ago 77 pass - Merge request !57Resolve #2774071: Fix Drupal.Scope.MethodScope.Missing, Drupal.NamingConventions.ValidFunctionName.NotCamelCaps → (Open) created by zniki.ru
- last update
over 1 year ago 77 pass - Merge request !58Resolve #2774071: Fix Drupal.Commenting, DrupalPractice.Commenting, Drupal.Files.LineLength.TooLong phpcs. → (Open) created by zniki.ru
- last update
over 1 year ago 77 pass - last update
over 1 year ago 77 pass - last update
over 1 year ago 77 pass - 🇷🇺Russia zniki.ru
Nikolay Shapovalov → changed the visibility of the branch 2774071-fix-coding-standard to hidden.
- last update
over 1 year ago 77 pass - last update
over 1 year ago 77 pass - last update
over 1 year ago 77 pass - last update
over 1 year ago 77 pass - 🇷🇺Russia zniki.ru
Nikolay Shapovalov → changed the visibility of the branch 2774071-fix-comments to hidden.
- last update
about 1 year ago 78 pass - 🇷🇺Russia zniki.ru
Nikolay Shapovalov → changed the visibility of the branch 2774071-fix-comments to active.
- last update
about 1 year ago 78 pass - last update
about 1 year ago 78 pass - last update
about 1 year ago 78 pass - last update
about 1 year ago 78 pass - last update
about 1 year ago 78 pass - Merge request !64Resolve #2774071: Squiz.ControlStructures, Squiz.WhiteSpace phpcs rules → (Open) created by zniki.ru
- last update
about 1 year ago 78 pass - last update
about 1 year ago 78 pass - 🇷🇺Russia zniki.ru
I create branch 2774071-fix-cs-base-branch with phpcs.xml file and exclude all the rules that has any violation. Also gitlab ci should fail when there is phpcs found.
I rebased all existing MRs on this branch, and remove rules from phpcs that exists in this branch.
My suggestion to Merge base 2774071-fix-cs-base-branch to 8.x-1.x, in that case MR diff will be more transperent, right now it's impossible to see what rules will be fixed in the MR. But I add rules to MR description.
At the moment there are violations that still need to be fixed:
<rule ref="Drupal"> <exclude name="Drupal.Files.LineLength.TooLong"/> <exclude name="Drupal.NamingConventions.ValidVariableName.LowerCamelName"/> <exclude name="Drupal.Semantics.FunctionT.BackslashSingleQuote"/> <exclude name="Generic.PHP.UpperCaseConstant.Found"/> <exclude name="PSR2.Namespaces.UseDeclaration.SpaceAfterLastUse"/> </rule>
- last update
about 1 year ago 78 pass - Status changed to Needs work
about 1 year ago 1:14pm 22 January 2024 - last update
about 1 year ago 78 pass - Merge request !65Resolve #2774071: Drupal.NamingConventions, Drupal.Semantics.FunctionT, Generic.PHP.UpperCaseConstant → (Open) created by zniki.ru
- last update
about 1 year ago 78 pass - Status changed to Needs review
about 1 year ago 1:33pm 22 January 2024 - Status changed to Needs work
about 1 year ago 10:00am 13 April 2024 - 🇨ðŸ‡Switzerland berdir Switzerland
Not sure what to do with this.
Each of those MR's will conflict as they all change the phpcs and gitlab config.
I'd rather not change those files, including the allow fail. I'd rather rely on the code quality difference, which gitlabCI I think is still working on properly supporting. Then we can see if the situation gets worse or improves in a MR.
drupal.org issues are also not well suited to merge multiple merge requests per issue. We should either split them into separate issues or combine it.
- First commit to issue fork.
- 🇫🇷France dydave
DYdave → changed the visibility of the branch 2774071-fix-cs-base-branch to hidden.
- 🇫🇷France dydave
DYdave → changed the visibility of the branch 2774071-visibility-array-indention to hidden.
- 🇫🇷France dydave
DYdave → changed the visibility of the branch 2774071-comments-switch to hidden.
- 🇫🇷France dydave
DYdave → changed the visibility of the branch 2774071-null-coalesce to hidden.
- 🇫🇷France dydave
DYdave → changed the visibility of the branch 2774071-fix-cs-squiz to hidden.
- 🇫🇷France dydave
DYdave → changed the visibility of the branch 2774071-use-fix-empty-line-fix to hidden.
- 🇫🇷France dydave
DYdave → changed the visibility of the branch 2774071-left-violations to hidden.
- 🇫🇷France dydave
DYdave → changed the visibility of the branch 2774071-fix-comments-debug1 to hidden.
- Status changed to Needs review
11 months ago 7:29am 25 May 2024 - 🇫🇷France dydave
Quick follow-up on this issue:
A few additional commits were added to the current merge request MR!58 with a few comments, see above at #58.
But mostly, at this point:
Last build on MR!58: https://git.drupalcode.org/issue/token-2774071/-/pipelines/181772- The PHPCS job now seems to be passing ✅
PHPCS job: https://git.drupalcode.org/issue/token-2774071/-/jobs/1684708
- The PHPUnit Tests are passing as well 🟢 \o/
PHPUnit job: https://git.drupalcode.org/issue/token-2774071/-/jobs/1684713
These changes should be harmless enough, without any change in module's code (only doc comments) so they shouldn't take too long to review and would help enforcing validation of coding standards for other pending merge requests.
We would greatly appreciate if a maintainer or someone with write permission could take a look at ticket's merge request MR!58 and let us know if there would be any more work needed.
Feel free to let us know if you have any questions or concerns on merge request MR!58 or any aspect of this ticket in general, we would surely be glad to help.
Thanks in advance for your feedback and reviews. - The PHPCS job now seems to be passing ✅
- Status changed to Needs work
11 months ago 8:42am 28 May 2024 - 🇫🇷France dydave
Sorry about that: I got a bit confused here with the number of merge requests in this ticket and didn't really understand what had been done...
It appears the merge request that I modified above (#59 only fixes part of the issues : the ones related with the doc comments blocks mostly, see :
https://git.drupalcode.org/project/token/-/merge_requests/58/diffs#diff-...As per #49 could someone please help creating some kind of "binder", with all the merge requests from this ticket merged into a single one, without a
phpcs.xml
file?
(without excluding any error from validation)Maybe checking out each branch locally and merging it into a new one ("binder") would allow retaining every commit that was made for every merge request created in this issue.
There will mostly likely be some conflicts to be fixed as well when the various merge requests are combined.Any help or feedback would be greatly appreciated.
Thanks in advance !
- 🇨ðŸ‡Switzerland berdir Switzerland
The patch in the issue I closed was 100kb. I will most likely never be able to commit enough time to review such a MR in full and commit it.
Separate issues, as mentioned without .gitlab-ci.yml and phpcs.xml changes (except adopting to use core phpcs.xml, see https://git.drupalcode.org/project/redirect/-/merge_requests/36), would be preferred I think and have a chance to get merged.