🇪🇸Spain @fjgarlin

We need to create follow-ups, but the current MR can be merged.

These two core issues talk about using @phpstan-type:
- 📌 Forbid limited length primary and unique keys, allow only in indexes Needs work
- 📌 Make Drupal\Core\Database\Database type strict and PHPStan L10 compliant Active

I'm not sure about the implications with this one.

Navigation might be real tricky.

Take as example these two files from core:
- README.md
- composer/Plugin/Scaffold/README.md

We could view these files:
1. Locally loading from the text editor ("drupal" is in the root folder)
2. Locally loading from the text editor ("drupal" is in the root/web folder)
3. Via git.drupalcode.org from "drupal" root page: https://git.drupalcode.org/project/drupal/-/blob/11.x
4. Via git.drupalcode.org from the "md" file page: https://git.drupalcode.org/project/drupal/-/blob/11.x/README.md

If we set the link from the first file to the second like:
- "/composer/Plugin/Scaffold/README.md" (1 ✅, 2 ❌, 3 ❌, 4 ❌)
- "composer/Plugin/Scaffold/README.md" (1 ❌, 2 ✅, 3 ❌, 4 ✅,)

If we set the link from the second file to the first like:
- "/README.md" (1 ✅, 2 ❌, 3 ❌, 4 ❌)
- "../../../README.md" (1 ❌, 2 ✅, 3 ❌, 4 ✅,)

The first syntax is the ideal and most simple, but it only works locally when drupal core is the root folder. This will not work if trying to jump between MD files in GitLab or Github.

So, the only alternative might actually be to provide what the api.drupal.org will be (eg: https://api.drupal.org/api/drupal/README.md, and this issue will be used to add the branch suffix "/11.x"). This will make navigation from 1, 2, 3, 4 jump to the api.drupal.org site, but it will always work.

This might be fixed by 🐛 Bugs with phpdoc data type display, especially for array types and shapes Active

This might be fixed by 🐛 Bugs with phpdoc data type display, especially for array types and shapes Active

This might be fixed by 🐛 Bugs with phpdoc data type display, especially for array types and shapes Active

Highlevel documentation pages might change into markdown: 📌 Use markdown for high-level documentation Active

I believe this is fixed. If you look for an interface, only one suggestion appears and it takes you to the right place.

If you look for "aliasedMethodName", you will find information.

Copying the reply that I gave in ✨ methods that are imported from traits as aliases can't be searched for Active .

--

Each class could alias a method differently. I don't think it needs storing, it just needs cross-linking, and that works.

Note that when the alias is used, like in here https://api.drupal.org/api/drupal/core%21profiles%21demo_umami%21tests%2..., the site already provide links to the actual unaliased function.

It also shows the alias information in the table below. You can see that same information in here too: https://api.drupal.org/api/drupal/core%21tests%21Drupal%21Tests%21Browse...

Also, using the global search finds the files/elements that use the alias.

I believe that this is correctly parsed in the newer versions.

https://api.drupal.org/api/drupal/core%21lib%21Drupal%21Core%21Config%21...

A theme could override this template if needed in any case.

I think the initial issue no longer occurs after the last round of fixes.

This is not adopted by core at the moment. Something that is already adopted is phpdoc data types, and we are implementing support for that here 🐛 Bugs with phpdoc data type display, especially for array types and shapes Active .

I'll close this for now. If core ever adopts the phpstan annotation format, then we can reopen.

Each class could alias a method differently. I don't think it needs storing, it just needs cross-linking, and that works.

Note that when the alias is used, like in here https://api.drupal.org/api/drupal/core%21profiles%21demo_umami%21tests%2..., the site already provide links to the actual unaliased function.

It also shows the alias information in the table below. You can see that same information in here too: https://api.drupal.org/api/drupal/core%21tests%21Drupal%21Tests%21Browse...

Also, using the global search finds the files/elements that use the alias.

We no longer parse the "vendor" folder. Closing as outdated.

All these references are to funcitons in the D7 codebase that no longer exist in the new version.

If this is still an issue, please re-open with URLs and steps to reproduce. Closing as outdated.

I've changed the value now. I will revisit this by the end of the week to see if the old entries were removed.

My bad, the code is deployed but I never changed the settings so it starts deleting them:

I will adjust the settings and deploy the new configuration.

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 →