Problem/Motivation

Drupal.org projects should use GitLab CI pipelines. Drupal CI is now legacy.

Steps to reproduce

Proposed resolution

Remaining tasks

User interface changes

API changes

Data model changes

Issue fork comment_delete-3390146

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:

  • 3390146-gitlab-ci Comparechanges, plain diff MR !7
    • 1 hidden branch
  • 2.x Comparecompare
About issue forks

Comments

bluegeek9 created an issue. See original summary.

bluegeek9 opened merge request !7

bluegeek9’s picture

bluegeek9 commented
Title: GitLan CI » GitLab CI
Assigned: bluegeek9 » Unassigned
Status: Active » Needs review
bobooon’s picture

bobooon commented
Status: Needs review » Postponed

This project doesn't have any tests written as of now. I'm all for adding the template file whenever some tests are written.

bluegeek9’s picture

bluegeek9 commented
Status: Postponed » Needs review

Hi @robphillips,

I understand if you want to hold off until there are tests. I updated the pipeline to not run phpunit and added drupal-check to the validation stage.

I may add a unit test for permissions. If I find a bug, I'll add a test for the bug.

This is a minor issue drupal-check found: #3391949: Minor item found by drupal-check.

The pipeline has three stages: composer install, validation, and test. By default, validation job may fail, but the pipeline will still be a success, and test job must pass. We can change this to require the validation jobs to pass for the pipeline to pass.

I recommend requiring the validation jobs to pass.

bobooon’s picture

bobooon commented
Status: Needs review » Needs work

This project is not using drupal-check for code validation. In fact, the MR needs to be revised to resemble the official Drupal template. https://git.drupalcode.org/project/gitlab_templates/-/blob/1.0.x/gitlab-...

This commit (https://git.drupalcode.org/project/comment_delete/-/merge_requests/7/dif...) should be opened as a separate ticket.

bluegeek9’s picture

bluegeek9 commented
Status: Needs work » Needs review

@robphillips,

I reverted the change to comment_delete.module.

bobooon’s picture

bobooon commented
Status: Needs review » Needs work

Looks better. Although the template doesn't exactly match the recommended default template. I don't believe there's currently a benefit to enabling the SKIP_PHPUNIT option. Can the default template be simply copied into place for now?

bluegeek9’s picture

bluegeek9 commented
Status: Needs work » Needs review

I reverted the pipeline to the default. I added tests. I moved the permission generator into a different class; it was easier to test as separate classes.

I appreciate your work on the delete comment project, @robphillips.

bluegeek9’s picture

bluegeek9 commented

PHPStan was added to the default workflow, by Drupal.org maintainers.

I updated the pull request to resolve the one issues PHPStan found.

https://git.drupalcode.org/project/comment_delete/-/merge_requests/7/dif...

bluegeek9 changed the visibility of the branch 2.x to hidden.