Problem/Motivation

Verified that now we can Implement GitLab CI on Drupal.org to validate PHPCS among other validations

The project Tour UI implemented. E.g.:

Proposed resolution

Issue fork modal_page-3392290

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

renatog created an issue. See original summary.

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

keszthelyi opened merge request !62

keszthelyi opened merge request !63

keszthelyi’s picture

keszthelyi commented

Hi @renatog

If you don't mind, I started implementing this, and added the .gitlab-ci.yml in 2 MRs: for 5.0.x and 6.0.x.

I think it would be important to set this up and run composer install and the tests before merging MRs.

The issue I found and reported today with the js_cookie version restriction would have been caught by CI. See composer install failing on 6.0.x because of this issue:
https://git.drupalcode.org/issue/modal_page-3392290/-/jobs/7205469#L83

To demonstrate my fix, I temporarily cherry picked that commit on the 3392290-gitlab-ci-6.0.x branch, and now the composer job passes:
https://git.drupalcode.org/issue/modal_page-3392290/-/jobs/7205471
(this commit can be removed from here of course, just added for the demonstration)

It's a work in progress, I'm planning to fix the PHPUnit tests here (as a minimum, I already fixed those in another MR), but if you agree the ideal would be to merge these before anything else, so the new MRs would already be tested on CI pipelines.

Also, it needs to be decided what Drupal versions the tests are running on of course. Currently it's 11 and 10, but this can be extended.

keszthelyi’s picture

keszthelyi commented
Status: Active » Needs work
renatog’s picture

renatog commented

Absolutely @keszthelyi

Thank you so much for look into this

It'll be awesome

keszthelyi’s picture

keszthelyi commented

Hi @renatog

I rebased 3392290-gitlab-ci-6.0.x branch (removed the cherry picked commit mentioned in #5).

I added the phpunit fixes to both branches, and the pipelines are now green.

I think the best would be to merge these like this, as a starter (they will provide a safety net already for ongoing MRs). I'd use separate issues for fixing the CSspell PHPCS, PHPStan etc issues as there are quite a few and not all of them are trivial to fix. Let me know what you think.

keszthelyi’s picture

keszthelyi commented
Status: Needs work » Needs review
renatog’s picture

renatog commented

I added the phpunit fixes to both branches, and the pipelines are now green.

Amazing, thank you so much @keszthelyi

I'd use separate issues for fixing the CSspell PHPCS, PHPStan etc

It makes sense for me

Okay, I’ll merge that into dev branch

renatog’s picture

renatog commented
Parent issue: » #3553252: Plan for the next Modal Releases
renatog’s picture

renatog commented
Status: Needs review » Needs work

Added few questions in the MR

renatog’s picture

renatog commented
Version: 5.0.x-dev » 6.0.x-dev

In the issue is marked as 5.0.x but the MR points to 6.0.x

Just adjusting the field here

keszthelyi’s picture

keszthelyi commented

Hi @renatog,

I added a reply to your comment in the MR, please check and let me know what you think.

Re: #13

In the issue is marked as 5.0.x but the MR points to 6.0.x

There is also an MR for 5.0.x (I just used the same issue for both).

renatog’s picture

renatog commented
Status: Needs work » Reviewed & tested by the community

I added a reply to your comment in the MR, please check and let me know what you think

Agreed with your point

Just resolved the thread on MR

There is also an MR for 5.0.x (I just used the same issue for both)

Got it! Makes sense

However, what do you think pointing to 5.1.x instead of 5.0.x?

Explanation:

  • I'm planning to deprecate the 5.0.x soon
  • I'll mark as "unsupported" at ~Jan/2026
  • So will be like:
    • 5.1.x -> stable, with support for D10 and D11
    • 6.0.x -> new features, experimental, etc
    • 5.0.x -> deprecated

So, people using 5.0.x could upgrade to 5.1.x

keszthelyi’s picture

keszthelyi commented

However, what do you think pointing to 5.1.x instead of 5.0.x?

Sure, you can merge it to 5.1.x. The reason I created that was because I found this existing issue and originally it was opened for 5.0.x, so the issue branch was automatically created from 5.0.x. (I was also considering creating an issue for 6.0.x but then I thought let's do it here)

I think it wouldn't hurt to merge it to both 5.1.x and 5.0.x btw, but up to you.