Problem/Motivation

Drupal.org now has a Gitlab CI template for automatically unit tests, coding standards tests, phpstan, and cspell.

Simply adding the template file to the project will start a suite of automated tests.

Issue fork rename_admin_paths-3442057

Command icon Show commands

Start within a Git clone of the project using the version control instructions.

Or, if you do not have SSH keys set up on git.drupalcode.org:

About issue forks

Comments

ptmkenny created an issue. See original summary.

ptmkenny opened merge request !13

ptmkenny’s picture

ptmkenny commented
Status: Active » Needs review

darvanen made their first commit to this issue’s fork.

darvanen’s picture

darvanen
he/him
Primary language English
Location Sydney, Australia
commented

I have set CI to skip phpcs for now I recommend making that a follow-up task if desired as the volume of errors is quite large and would only serve to delay this issue.

I fixed the CSPell errors by adding ignored words for names and two adjustments to other words - I checked that changing the name of the admin menu item doesn't cause any regressions in positioning on a site, it's ok.

PHPStan only had one error, I put a default return in the help hook to satisfy that.

Tests are all green so I'd say this is ready for review.

ptmkenny’s picture

ptmkenny commented

@darvanen Why do you think phpcs need to be disabled? This issue is for adding the GitLab template, not fixing all the tests.

I have been getting GitLab CI templates committed to several projects, and the way many maintainers have decided to do this is commit the template as one issue, and then commit fixes for each test (phpcs, cspell, etc.) as separate issues.

I feel like trying to fix all the tests in one giant issue makes it harder to commit, not easier.

darvanen’s picture

darvanen
he/him
Primary language English
Location Sydney, Australia
commented

@ptmkenny my experience with maintaners has been that they like to have all tests passing before committing anything, though that may be more prevalent in core than contrib.

Apologies for overriding your work so soon after it was submitted, I should have looked at the timestamp, I was just trying to help.

I'm happy to split out the the work into separate issues, I just don't want to be seen as credit-gaming.

darvanen’s picture

darvanen
he/him
Primary language English
Location Sydney, Australia
commented

Oh I didn't answer the first question:

I disabled the phpcs so that a follow-up could be made to turn the test on and fix the errors in a separate issue. That way the test suite in use runs green by default making it easy to spot problems in MRs. If we don't fix the other tests in this issue I'd recommend doing the same thing with those.

ptmkenny’s picture

ptmkenny commented

@darvanen Thanks for responding. The GitLab CI template is configured so that the icon on MRs is still green even if phpcs fails; to see the phpcs failures, you have to click the GitLab CI icon and look at the details of the tests. In fact, only phpunit failures will result in a red icon; otherwise, you get green. So there is no no reason to fix all the tests in a single issue to see green; only the unit tests have to pass.

For this reason, I have re-enabled the phpcs test as I think the information is useful for people who are evaluating the module. I also created a separate issue, #3444739: Fix coding standards issues.

ptmkenny’s picture

ptmkenny commented

@darvanen Thank you for fixing the tests; I'm happy to see more problems get fixed, and don't mind at all to see you add to the MR.

ptmkenny’s picture

ptmkenny commented

  • ptmkenny committed b427c71c on 8.x-2.x 
    Issue #3442057 by ptmkenny, darvanen: Add GitLab CI template
ptmkenny’s picture

ptmkenny commented
Status: Needs review » Fixed

Thanks for your help @darvanen, I am now a maintainer and will get a D10 + D11 releases done.

Status: Fixed » Closed (fixed)

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