Problem/Motivation

According to GitLab CI - FAQ,

The Drupal CI infrastructure will be disabled on July 1st, 2024, with some functionality already deprecated.

Now is a good time to enable GitLab CI testing.

Steps to reproduce

Proposed resolution

  1. Follow the instructions on GitLab CI,
  2. Fix problems reported by static analysis (phpstan and phpcs) or at least establish a baseline.

Remaining tasks

User interface changes

None

API changes

None

Data model changes

None

Issue fork conditional_fields-3423942

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

benjifisher created an issue. See original summary.

benjifisher opened merge request !41

benjifisher’s picture

benjifisher
he/him or they/them
Primary language English
Location Boston area
commented
Status: Active » Needs work

I set up GitLab CI. You can see the results on the MR: https://git.drupalcode.org/project/conditional_fields/-/merge_requests/41.

I fixed most of the errors and warnings reported by phpcs.

I added a baseline for phpstan, so that we can try to avoid new errors and eventually fix the existing ones. I used the one generated as an artifact from the GitLab CI run. But that does not seem to be working, so I am setting the issue status to NW.

benjifisher’s picture

benjifisher
he/him or they/them
Primary language English
Location Boston area
commented

I fixed the one problem reported by stylelint.

I would rather let someone more familiar with JS take a look at the eslint results.

I am still doing something wrong with phpstan.

D-XPERT made their first commit to this issue’s fork.

benjifisher’s picture

benjifisher
he/him or they/them
Primary language English
Location Boston area
commented

@D-XPERT:

Thanks for looking at the eslint errors. I left a few comments on the MR. If you have a chance, please respond to them. I am setting the issue status back to NW for that.

Your commit messages were brief. Could you say a little more? It looks as though the first commit might have been automated fixes, and the second commit was manual. Is that right?

Meanwhile, I have ignored the remaining phpcs error, fixed my broken phpstan configuration, and updated a test to match the changes in the code. I think this issue is almost done!

d-xpert’s picture

d-xpert commented

@benjifisher, thanks for the review. I will work on the suggestions.

benjifisher’s picture

benjifisher
he/him or they/them
Primary language English
Location Boston area
commented
Assigned: benjifisher » Unassigned
dqd’s picture

dqd
Location London | N.Y.C | Paris | Hamburg | Berlin
commented

Awesome work in here. 1+! Thanks for working on this important issue! Will follow and will be available for any question.

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

heddn’s picture

heddn
Primary language English
Location Illinois
commented
Status: Needs work » Reviewed & tested by the community

I felt the JS changes are too risky in this "enable gitlab ci" issue. We can resurrect them in a follow-up clean-up task. JS style linting fix stuffs do not break the build in gitlab, they are just thrown as warnings. Better to solve in a dedicated issue.

heddn’s picture

heddn
Primary language English
Location Illinois
commented
Status: Reviewed & tested by the community » Fixed

Thanks for all your contributions here. It made it really easy to commit things at the end.

heddn’s picture

heddn
Primary language English
Location Illinois
commented

Opened #3463216: Fix eslint errors as a hopefully much easier to review follow-up.

dqd’s picture

dqd
Location London | N.Y.C | Paris | Hamburg | Berlin
commented

Thanks for all your contributions here.

I would like to add my voice to that. Very much appreciated contribution. Thanks a million. And many many thanks to heddn for tracking and merging it so fast. And yes, good idea to split the eslint warnings to move on.

Status: Fixed » Closed (fixed)

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