Review all documentation pages

https://git.drupalcode.org/project/gitlab_templates/-/blob/main/docs/inf... needs fixing. It's just the first space.
Linked to https://project.pages.drupalcode.org/gitlab_templates/info/common/#viewi... (updated IS with it)

Consider trimming comments in https://git.drupalcode.org/project/gitlab_templates/-/blob/main/gitlab-c... and add links to the documentation page (related thread https://drupal.slack.com/archives/CGKLP028K/p1712807961399779).

Updated IS

On the front page https://project.pages.drupalcode.org/gitlab_templates/ we have Jobs followed by Basic Info. But in the sidebar the 'Get Started' item is first, which has all the basic info pages, the the 'Jobs' item. I think it would be better if the sequnce was the same on both?

Happy with that. The main menu structure is in the mkdocs.yml file.

Re #7, even better just to move the content within the index.md file. I think the sidebar menu makes sense for it to be in that order

even better just to move the content within the index.md file. I think the sidebar menu makes sense for it to be in that order

That's exactly what I was planning to do. Not delete anything just swap the two blocks in the index.md

Updated IS. I will work on the index.md then maybe one more page, then it can go for review and merge. I don't intend to do every page in one MR.

For info, following up a thread in the MR, the --ignore-path parameter does not accept multiple paths, and a request to implement it was turned down - see https://github.com/eslint/eslint/issues/9794 But it is OK as we can use multiple ignorePaths in the eslintrc.js file

Good find. I'm happy in any case having moved that fully to the eslintrc.js file.

The MR looks good. Not sure if you wanted to do some more changes to it or if it's ready for review.

I pushed one change since your last comment. It tidies up template.gitlab-ci.yml even more. Instead of duplicating the details of modifying 'ref' it now has a link to the doc page. Also I've moved the comment about D7 out from the actual executable lines, to make it clearer to read. Another benefit is that the new comment does not have the full d7 filename, which means we can search better for the use of d7 file (given that the gitlab search is a bit broken and we cannot detect precise strings including a comment #). I know there are lots of templates already in use with the full d7 filename in a comment, but this change will stop there being more of them.

I'm happy for you to review this and merge, so that the updated template becomes active. Then I can work on more pages in part 2.

I made a really minor comment in the MR, everything else look really good.

Thanks. I also responded in the MR. Will be nice to get this merged.

Thanks so much for these updates, they are great!! Merged.

Back to NW for part 2

I've updated the issue summary. The D7 additions are great, but I will modify the intro paragraph. I don't think we want the causal ending "thats it!" then proceed to explain lots more complex things.

Good point on #21 🙂

Feel free to edit as needed. I mostly wanted to add those bits in for D7.

I have moved the Composer paragraph up, then the dependencies details follow on logically from it. Also tightened and tidied a bit. I think this page is ready. But it would be worth reviewing/fixing another page before merging.

Thanks! I added a really small suggestion to the MR.

I'm happy to merge small iterations as well. Merging these MRs won't affect in any way "default-ref" or any of the tags, so it should also be fine.

As you marked it as "needs review", I think this part is RTBC. Do you want me to merge this in or do you have any other little improvements in mind? I'm happy to wait. Also, even if I merge this, this issue can remain open.

I had some previous notes about the PHPunit page, so I have added an update of that page to MR186

I'm happy with the changes. We can merge and, if needed, open additional MRs for future improvements. RTBC.

Merged the new MR. Marking as fixed, but feel free to re-open if we want to keep on improving pages.

I see you removed the anchor in jobs.md#skip-and-opt_in-variables. Did it not work correctly? It is hard to check the links when we don't actually publish the pages within the MR (and I don't have it mkdocs running locally).

It did not work, but then I found a way to make it work and committed here https://git.drupalcode.org/project/gitlab_templates/-/commit/d10995a46e0...

I needed to run it locally (https://project.pages.drupalcode.org/gitlab_templates/help/documentation...) to debug things a bit. It was the ./jobs.md, which should have been a ../jobs.md

Ah, I thought you had removed the anchor because I read the extra the commit in #30 first, then the commit in #31, and just assumed that they were done in that sequence, i.e. comment #31 was after comment #30. But actually the commit in #31 was before the commit in #30.

jonathan1055 → changed the visibility of the branch main to hidden.

I've created a new branch for the next set of doc pages review/updates, therefore setting issue to active, rather than let it get closed. I might not have time to do much for a few weeks, so wanted to keep this issue open.

Thanks for creating the branch. I made a small change to remove unnecessary space in the Drupal 7 page.

Linking this slack conversation: https://drupal.slack.com/archives/CGKLP028K/p1717120429511369

TL;DR: we should mention, around the OPT_IN variables, that the default values is what we recommend. Modules should only enable variants because they truly need them, as enabling them incurs in extra costs which might not be needed.

Two more things to do:

1. Somewhere on the 'home' page, or in the header, it would be nice to have a link back to https://www.drupal.org/project/gitlab_templates →
2. We have duplication in https://www.drupal.org/docs/develop/git/using-gitlab-to-contribute-to-dr... → with information that is going to change and we'll have an overhead in maintaining both versions. Do we need to look at what we want to keep in both places and what should be removed from the drupal.org page?

I added a link to the project homepage and some additional information about PHPUnit variants.

We have duplication in https://www.drupal.org/docs/develop/git/using-gitlab-to-contribute-to-dr → ... with information that is going to change and we'll have an overhead in maintaining both versions. Do we need to look at what we want to keep in both places and what should be removed from the drupal.org page?

Agree. We should remove all duplicated information or just link to the documentation site (if we still want to keep the section). The only block where I think duplicated information might be good is "How do I get started with GitLab CI?", because it contains the crucial information about setting the templates up in the first place and should be easy to find. Also, that info is unlikely to change as it is purely GitLab UI related.

Pushed a full review and overhaul of the PHPStan page, new version linked in the issue summary.

Thanks for the recent changes. I'll go with your lead on when to merge these improvements. As it's just documentation it's safe to merge at pretty much any point and then create new MRs for future improvements.

From #3400979-80: Is SYMFONY_DEPRECATIONS_HELPER: weak correct for contrib deprecation testing needs? → I have added the slight correction to PHPunit. I will do a review of one more page, then we can merge.

Looks like the page for the upgrade_status job needs to be added to the sidebar. As far as I can see that is the only job missing. I didn't check the other sections of the nav.

Just noticed something else: The cspell job doc says:

For .md files you can use the comment style [//]: # cspell:disable and [//]: # cspell:enable

For the project I'm switching to GitLab CI today, I decided to include README.md this time, so I had to ignore a word. And it doesn't work with the above-mentioned syntax as you can see here the comment is shown prominently at the top of the file in GitLab.

According to some blog post, you'd need to add parenthesis around the commented text. I didn't try that but opted to use the HTML comment style instead, because when reading the post I remembered that I had used that previously and it had worked. Indeed that hid the line in the GitLab output and the cspell job still succeeded:

1. Relevant commit
2. Rendered Markdown in GitLab
3. Successful job in the upstream project

And one more thing I was wondering about when reading the cspell documentation was: If I include my own dictionary, will the job use my dictionary on top of the Drupal Core's dictionary (that's what I would prefer) or will my dictionary replace Core's dictionary? I didn't yet try that or check the code in the template, but maybe, if you happen to know how that works already, it would be nice to add that as well.

Someone on twitter pointed out that the upgrade status job should be compared to the opt in test next major. Maybe that is useful for the docs too? See
https://www.hojtsy.hu/blog/2024-jul-05/continuous-forward-compatibility-... "How does this compare to running tests on the next major version?" section. @fjgarlin added on Slack:

We can clarify differences in the docs. It’s an individual job as opposed to a full variant and it can even help you with the fixes. Maybe this can be a comment in the open issue we have for docs improvements.

#45 addressed here: https://git.drupalcode.org/project/gitlab_templates/-/merge_requests/202...

#46 addressed here: https://git.drupalcode.org/project/gitlab_templates/-/merge_requests/202...

#46 - If you have additional dictionaries I think they'll all be added, as seen here: https://git.drupalcode.org/project/gitlab_templates/-/blob/main/scripts/.... I guess extra wording about this could be added in the "Full customization" section.

#47 addressed here: https://git.drupalcode.org/project/gitlab_templates/-/merge_requests/202...

I'd say we have a good number of changes to make another push. @jonathan1055, what do you think? Are you planning on adding something else?

As this is just documentation changes, I am going ahead and I will merge the MR and mark the issue as Fixed.
It can be reopened again and a new MR can be created or a new issue can be open for new improvements.

Thanks!!

Regarding the .md comment style in #46, the error was that the help text was missing quotes. The comment was still being accurately processed by cspell.
[//]: # cspell:disable is wrong
[//]: # "cspell:disable" is correct and the text is not rendered.

You can see this in Scheduler rendered README.md and the raw README.md source

In the next update of documentation pages, I may add back the alternative, with quotes, so that we can show both types.

Sounds good. Thanks for getting back on that @jonathan1055. Having both alternatives would be good and as you say, it can go in the next iteration of improvements.

Opened https://git.drupalcode.org/project/gitlab_templates/-/merge_requests/244 for a new round.

As per agreement on Slack I've suggested some improvements to the newly published 'variants' page, and also added a few other documentation enhancements. One thing that I've noticed, and would like to suggest as a recommendation going forward, is the order of the variants/opt_in variables. It is helpful to always refer to these in the same order, in various blocks of code, comments, issues etc, to make cross-reference quicker and easier. We seem to have settled on a good ordering in many locations, and this is:

    OPT_IN_TEST_CURRENT
    OPT_IN_TEST_MAX_PHP
    OPT_IN_TEST_PREVIOUS_MINOR
    OPT_IN_TEST_PREVIOUS_MAJOR
    OPT_IN_TEST_NEXT_MINOR
    OPT_IN_TEST_NEXT_MAJOR

So I have moved a few lines in the new Variants page to match this order. It saves brain power and speed of grokking when looking at two lists, to have them in the same sequence. I propose we stick to this order for future pages. I will also fix any places where things are not in this order, as and when I see them.

The changes are ready for review.

Thanks for the additional changes. I just merged the MR.
I agree that we should stick to that order if possible.

Marking this as Fixed but do feel free to re-open it as well and create a new MR if needed.

I have just checked in #Adding a pipeline status badge and the link is shown as plain text.
https://project.pages.drupalcode.org/gitlab_templates/info/common/#addin...
It was rendered as a clickable active link on preview when editing and when I viewed the file directly in the MR. So I did not realise that it would not render as active in the real page. I wonder if there is anything we can do to avoid this error? Should we have a new job in code-check that tries to find plain http links in doc pages that are not written with the [ ]( ) syntax?

Unintended status change

Fixed the link here: https://git.drupalcode.org/project/gitlab_templates/-/commit/b0738b52f15...
I also fixed these earlier: https://git.drupalcode.org/project/gitlab_templates/-/commit/3f3484bed6d...

That could be a good check for the syntax checks that we do for the documentation. It can be a separate issue in my opinion.

Thank you for fixing those links.

I have created #3469949: Internal code check - find non-active documentation links →

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