🇪🇸Spain @fjgarlin

fjgarlin → changed the visibility of the branch 3327584-marketplace-counts-modern to hidden.

fjgarlin → changed the visibility of the branch 3327584-dedupe-credits to hidden.

Thanks for opening the issue. I was going to suggest that (opening the issue in the project). It might have been a conscious decision, but if it isn't, we can help them keep the simple version.

I did something similar on August 24th via slack with the "pdf_api" maintainer (https://git.drupalcode.org/project/pdf_api/-/commit/e2afe127128845cd4453...).

Maybe we need to make something more obvious in https://project.pages.drupalcode.org/gitlab_templates/info/setup/ .
For example, in https://www.drupal.org/docs/develop/git/using-gitlab-to-contribute-to-dr... → we have:

Note: You do NOT need to copy in any of the other 'include' files, GitLab will reference those directly from the template.

Merged! It will go with the next deployment.

fjgarlin → made their first commit to this issue’s fork.

Merged. Thanks!

Merged.

Test passed now.

Merged! I will test locally this and the other merged issues and then deploy.

Merged.

The code changes you did look good. RTBC on that part.

As for the "shellcheck" issue. There were some updates in the runners last week, which I think should not affect this. Maybe the image we use was updated in "drupalci_environments", I'm not sure.

I saw this here: https://support.atlassian.com/bitbucket-cloud/kb/error-404-failed-to-fet...

I changed the image from 8.1 to 8.3 and that seems to work well. So RTBC.

This looks good but the tests are failing at the composer stage. I'm going to merge the RTBC issues and then rebase this MR to see if that's enough.

The api.drupal.org is on the latest Drupal 10.5.x stable version and it's on PHP8.1. That's exactly what's tested in the "previous major" variant in the pipelines: https://git.drupalcode.org/issue/api-3534214/-/pipelines/546323

The code looks good.

Thanks for the suggestions, I've applied those.

I don't really think we need to do anything about BC in this case. We are changing the behaviour very slightly, for something that probably makes even more sense, and we are documenting it. One could even argue that using that value was a bug. It's similar to what we did here: 📌 Use stable core versions Active .

We can post a message in #slack or even create a change record, tho I don't think that the latter is needed.

fjgarlin → changed the visibility of the branch 3327584-include-sa-param to hidden.

I really think that it's more confusing to have the security version rather than the latest stable previous minor.

I added the new variable, made the needed checks.

First test with wrong value (so the script detects it): https://git.drupalcode.org/project/gitlab_templates/-/jobs/5854307
Second test with the correct value: https://git.drupalcode.org/project/gitlab_templates/-/jobs/5854320

I am keeping all the logic for the other variable as we do it for a few other values, even if they are not used within the templates.

This is ready for review.

fjgarlin → changed the visibility of the branch 3327584-small-fixes to hidden.

To be honest, if you check the changelog, this has probably been the longest without producing a tag and moving default-ref.

Having said that, I tagged yesterday 1.10.0 before you created the issue, and I was planning to make it the default-ref today (as yesterday it was the end of my day).

I will announce the change in the #gitlab channel, so you should be seeing the newer versions from today 🙂

Hi,

We have exactly what you are describing running daily: https://git.drupalcode.org/project/gitlab_templates/-/blob/main/.gitlab-...

We use instance wide variables with the recommended release.

In this case, we delayed the push to all contrib of the last tag (or main) due to some breaking changes introduced in core 11.2 which would have made almost all contrib tests start failing. We did some fixes, core did some fixes, and contrib was unaffected thanks to not releasing the latest versions. We normally tag and update default-ref more frequently but in this case we wanted to be sure before doing the switch.

See what when into the latest tag to have an idea: https://git.drupalcode.org/project/gitlab_templates/-/blob/main/CHANGELO...

The last core security release is 11.1.5, which is what we have and check in the code: https://git.drupalcode.org/project/gitlab_templates/-/jobs/5829882#L56

So, all in all, it really seems to me that you just want what “main” offers, which is latest versions.

We’d rather wait and be cautious before releasing a core version jump to all contrib, specially knowing that it would cause issues and generate even more confusion.

This is now merged.

I am merging the other one now. This is RTBC.

We missed something really small in an if condition in the last MR. This one fixes it: https://git.drupalcode.org/project/drupalorg/-/merge_requests/370/diffs

fjgarlin → changed the visibility of the branch 3327584-debug-org-rank to hidden.

fjgarlin → changed the visibility of the branch add-project-breadcrumb to hidden.

100% ok. We will try to keep them aligned (by cherry picking) when it makes sense. If it doesn't make sense, it's better to keep things clean than add extra clutter.

We currently do not check these files in phpcs: https://git.drupalcode.org/project/gitlab_templates/-/blob/main/assets/p...

We also ignore these in the cspell job.

Closing the issue as the project is already doing what is requested.

Looks good. RTBC pending the removal of the temporary code. It will also need a follow up commit on the GTD to test this new variable.

The code looks good and there are new supporting tests. RTBC.

This is now merged. Thanks for the fix and the review.

It will go with the next deployment.

I reviewed the code and tested everything locally and it all works as expected. Thanks!

Merging this. It will go with the next deployment to the site.

fjgarlin → made their first commit to this issue’s fork.

This is now merged and will go with the next deployment. Thanks!

fjgarlin → made their first commit to this issue’s fork.

I left some questions / suggestions in the MR. NW based on that.

There are some small phpcs issues too (I made pipelines fail when any lint step fails in a recent commit). I can work through those if the main questions above are fixed.

As we are only using this feature for MD files and we've tested on non-prod enviroments succesfully, I'm merging this.

It will go with the next deployment.

The MR looks good and simple. RTBC.

Pipeline after the above change: https://git.drupalcode.org/issue/gitlab_templates-3533957/-/pipelines/54...
Job that was failing is now passing again, as expected: https://git.drupalcode.org/project/keycdn/-/jobs/5816533

Thanks for noticing! Happy either way. It can go with the other one given that it's the most active issue at the moment.

Error in stylint: https://git.drupalcode.org/project/keycdn/-/jobs/5810304

Corepack must currently be enabled by running corepack enable in your terminal. For more information, check out https://yarnpkg.com/corepack.

100%, it's great that we found a case to do it. That will improve test coverage.

I also triggered the downstream pipelines for KeyCDN and Decoupled pages as they both have stylelint job: https://git.drupalcode.org/issue/gitlab_templates-3533957/-/pipelines/54...

Good one. Merging.

We can see how composer is happy now: https://git.drupalcode.org/issue/wisski_default_data_model-3534507/-/job...

First run shows the composer error: https://git.drupalcode.org/issue/wisski_default_data_model-3534507/-/pip...

In VersionParser.php line 526:
                                                                               
  Could not parse version constraint 3495302-warning-trying-to: Invalid versi  
  on string "3495302-warning-trying-to"                 

Now another commit to fix that.

This is due to this line: https://git.drupalcode.org/project/wisski_default_data_model/-/blob/1.x/...

That's referencing a fork branch, but we are not telling composer to look at the fork location.

Adding GitLab CI to this project would alert of this.

The format above was validated. We can work on this.

Great. I updated the issue description in ✨ Use default branch slug on file paths without it Active , so we can continue there.

I created an MR with a possible fix for this. Please review.

fjgarlin → created an issue.

Thanks for the review.

That YN00... can be ignored.

I think that the new snippet should capture the folder we're in at that moment and then cd backinto it after running yarn install.

Back to NW based on that. Would you be ok doing that small change? Otherwise I could too.

I’m still unsure about what gitlb_templates can do for it. We don’t provide tests for contrib modules.

The above mechanisms (use a different image, install/uninstall extensions via before_script) should help any maintainer with this.

If a new image might be needed then “drupalci_environments” is the project, but i don’t think that we’ll be creating images with really small package variations when they can be manipulated in “before_script” sections.

You link a core issue, so we should probably wait for resolution there and see if anything is needed.

Can you explain what you think that could be done in this project?

The extension comes from the base image utilized: https://git.drupalcode.org/project/drupalci_environments/-/blob/dev/php/...

Note that you can use different images if wanted: https://project.pages.drupalcode.org/gitlab_templates/info/customization...

You could also manipulate packages / extensions in the “before_script” section of the job: https://project.pages.drupalcode.org/gitlab_templates/info/customization...

I don’t think we need to do anything in the templates for this case. Hopefully the above links will help you achieve what you need.

The code is ready for review.

Sample file where I introduced an image via html markup.

fjgarlin → created an issue.

If links contain the path of where the file is, we could scan the code and replace appropriately.

eg: this is a [link](/core/themes/starterkit_theme/README.md) that should work (on markdown)

Could be transformed to:
this is a [link](/api/drupal/core%21themes%21starterkit_theme%21README.md/11.x) that should work (on the api site)

Again, first agreement, then we can repurpose issue ✨ Use default branch slug on file paths without it Active to match what we need here.

Thanks! I've now merged this.

Merged. Thanks!

It'd be great to agree on the above first, and then we can do ✨ Use default branch slug on file paths without it Active

fjgarlin → created an issue.

Defo, agreeing on some conventions can make everything easiler.

The current MD files are all accessed the same way tho:
- https://api.drupal.org/api/drupal/core%21themes%21starterkit_theme%21REA...
-- https://api.drupal.org (host)
-- api (api project prefix)
-- drupal (project slug)
-- core%21themes%21starterkit_theme%21README.md (url-encoded path to the file)
-- 11.x (branch slug)

That goes for any file path.
We could, in a follow-up, not require the branch slug, check if the path is a file path, and in that case, add it automatically.

That way, all files would have https://api.drupal.org/api/drupal/PATH%21TO%21FILE.md for the latest/default branch, which I think it might be usable and easy enough to cross-link things.

Yes please, try with scheduler. That's got way more tests.

I am assuming that "node_modules" is never needed for PHPUnit tests. In any case, this is opt-in and it is documented how to add it to any job, so it shouldn't be an issue. But yes, let's test scheduler with this feature turn on and off and see the results.

Markdown files are now parsed and displayed by api.drupal.org:
- https://api.drupal.org/api/drupal/core%21themes%21starterkit_theme%21REA...
- https://api.drupal.org/api/drupal/README.md/11.x

I created a follow- to allow html markup rendering from trusted projects (aka core): 📌 Allow html rendering of trusted projects in markdown files Active

But in the meantime, core can already test/experiment how MD files look like in the api.drupal.org site.

fjgarlin → created an issue.

Yeah, for now we'd keep it in the "variables" so it's exposed in the Web UI form, but a possible follow-up might change that. I think this should at least help with the disruption.

There are some other issues, like 📌 Exclude node_modules folder from artifacts Active , which might be needed as part of the new release, to give maintainers options in case they run into issues. I know that core is also working on improving things on their end.

@jonathan1055 - I was able to retrigger GTD pipelines again, thanks!
https://git.drupalcode.org/project/gitlab_templates/-/pipelines/538928

I did the above.

The new variable's default value makes everything behave the same as up until now, so no change will be pushed unless opted in, and we've documented it as well.

We don't need the change record in this case, so I'll remove that, but the MR is ready for review again.

Perhaps we can go forward with this issue as opt-in, that way modules like XB could just turn a variable on and use this approach.

larowlan → credited fjgarlin → .

I’m going to change the priority of this purely for queue sorting purposes right now.

We’re working through some of the other critical issues and most of them are fixed now, so maybe we can squeeze this as this format us adopted by core and actually breaking some pages.

Most jobs aren’t run if there aren’t files of certain type present (eg: no phpunit job will trigger if there are no php files), so that’s already taken.

With the new limit, let’s see how it holds.

This solution can be a plan B if needed or even useful if people have similar issues, so they can customize their integrations.

So I’ll close it for now.

This being such a small MR and core having changed the logic to optimize slow/fast test, I think it's worth merging. We can always revert if we see that it is disrupting contrib, but it shouldn't based on some of the tests above.

That issue above is fixed already, where are we on this one?

I think that setting the variable with the expected default value that will avoid disruption in contrib might be the quickest/easiest in this case.
Core does exactly the same thing https://git.drupalcode.org/project/drupal/-/blob/11.x/.gitlab-ci.yml#L70, so I think this is good for a RTBC.

Otherwise we are blocked on pushing 11.2 as the new default-ref for all contrib.

Agree to keep things simple. Thanks for confirming.

Draft CR: https://www.drupal.org/node/3533973 →

MR in place and ready for review. I triggered KeyCDN and Decoupled pages but can't do the GTD (I was removed from the maintainers in order to test #3426311: Allow testing documentation pages via MRs → ).

fjgarlin → created an issue.

The above comment shows the removal of the template from bluecheese, and the below commits the update of the template in the contrib module to use bluecheese:
- https://git.drupalcode.org/project/contribution_records/-/commit/1d18971...
- https://git.drupalcode.org/project/contribution_records/-/commit/f85db9c...

Follow-up created 🐛 Check if entities are revisionable before creating revision Active

fjgarlin → created an issue.

I decided to create a new tag for this feature: https://www.drupal.org/project/bulk_edit_terms/releases/2.1.0 →

Jump from 2.0.0 to 2.1.0 because it's a new feature.

This is a great addition. Great job! RTBC.

Merged. I will follow up clean up duplicated templates and updating dependencies.

Good call. I've actually been meaning to remove the twig templates from "contribution_records" as we have them in "bluecheese".

On latest D10:

I added the following to the field.storage.node.field… file:

indexes:
  field_last_status_change__value:
    - value

Then see before and after:

$ ddev drush sql:query "show indexes from node__field_last_status_change;"
node__field_last_status_change  0       PRIMARY 1       entity_id       A       4875    NULL    NULL            BTREE           
node__field_last_status_change  0       PRIMARY 2       deleted A       4875    NULL    NULL            BTREE           
node__field_last_status_change  0       PRIMARY 3       delta   A       4875    NULL    NULL            BTREE           
node__field_last_status_change  0       PRIMARY 4       langcode        A       4875    NULL    NULL            BTREE           
node__field_last_status_change  1       bundle  1       bundle  A       2       NULL    NULL            BTREE           
node__field_last_status_change  1       revision_id     1       revision_id     A       4875    NULL    NULL            BTREE           

$ ddev drush cim
+------------+---------------------------------------------+-----------+
| Collection | Config                                      | Operation |
+------------+---------------------------------------------+-----------+
|            | field.storage.node.field_last_status_change | Update    |
+------------+---------------------------------------------+-----------+

 Import the listed configuration changes? (yes/no) [yes]:
 > 

 [notice] Synchronized configuration: update field.storage.node.field_last_status_change.
 [notice] Finalizing configuration synchronization.
 [success] The configuration was imported successfully.

$ ddev drush sql:query "show indexes from node__field_last_status_change;"
node__field_last_status_change  0       PRIMARY 1       entity_id       A       4875    NULL    NULL            BTREE           
node__field_last_status_change  0       PRIMARY 2       deleted A       4875    NULL    NULL            BTREE           
node__field_last_status_change  0       PRIMARY 3       delta   A       4875    NULL    NULL            BTREE           
node__field_last_status_change  0       PRIMARY 4       langcode        A       4875    NULL    NULL            BTREE           
node__field_last_status_change  1       bundle  1       bundle  A       2       NULL    NULL            BTREE           
node__field_last_status_change  1       revision_id     1       revision_id     A       4875    NULL    NULL            BTREE           
node__field_last_status_change  1       field_last_status_change_field_last_status_change__value        1       field_last_status_change_value  A       4875    NULL      NULL            BTREE           

I didn't get any exceptions or warnings or errors in the status report.

This MR left, prod on the right.

In the feedback round, it was mentioned that font size was still too big. Are we addressing this here?

I agree that this is not the same as 🐛 Fatal error after updating to toc_api:^2 Active .

This issue belongs to this queue and. can be fixed as suggested in #14.

I think that the reported error is different from this one (this one was only about the blockAccess method return type), so a follow-up issue, different from this one, is probably better in my opinion.

https://api.drupal.org/api/drupal/core%21lib%21Drupal%21Core%21Render%21... works now.

I'll merge this and deploy with the other round of issues.

Also in the logs right before that, which is more likely to be the culprit:

Warning: Undefined property: stdClass::$search_name in Drupal\api\Formatter::makeMatchLink() (line 2285 of /app/web/modules/contrib/api/src/Formatter.php)
#0 /app/web/core/includes/bootstrap.inc(166): _drupal_error_handler_real()
#1 /app/web/modules/contrib/api/src/Formatter.php(2285): _drupal_error_handler()
#2 /app/web/modules/contrib/api/src/Formatter.php(1919): Drupal\api\Formatter::makeMatchLink()

The MR above should fix this case.

I'm merging this as it is. Once it is fully deployed to production and we start seeing MD files pages, we can create follow ups if needed.

It'll be deployed in the next round of deployments. I'll follow up with a link here once it's in production.