Order of keywords in jobs

The above order is what we have mostly been following, but I thought it worth taking a look to see what other developers do, and/or if there is consensus

Tim Plavec on Medium suggests an order that is fairly close to what we are already are doing:
https://tinplavec.medium.com/gitlab-ci-cd-best-practices-i-recommend-aft...

extends
stage
needs
rules
tags    << we do not use tags 
image
environment << we do not use 
variables
script
allow_failure
artifacts
cache  << we do not use 

He does not have 'services' in his list. I have not found any other recommended orderings.

His idea of having artifacts at the end sounds quite good. They are logically produced by the script, at the end of the job.

It would also be quite easy to write a simple script to verify the order in our two files, but Gitlab Templates own .gitllab-ci.yml, and run this as part of our code checks. We won't be adding too many new items very often, but the script would help us to get the order right, and keep it so.

Initial commit, with a php script to check the keyword order. It failed as intended, because I have not moved any of the keywords, and also left some undefined, to demonstrate the output.

Tangentially related, I noticed that the project's .gitlab-ci.yml is being ignored by the eslint check. We do not have a .eslintignore or .prettierignore file, and even using --no-ignore locally this file is still not tested. I adde --debug to check what happens in the pipeline, and it is ignored just like when running locally, and I don't know why.

2025-04-12T16:55:27.106Z eslintrc:ignore-pattern Check {
  filePath: '/builds/issue/gitlab_templates-3518793/.gitlab-ci.yml',
  dot: false,
  relativePath: '.gitlab-ci.yml',
  result: true
}
2025-04-12T16:55:27.106Z eslint:file-enumerator Yield: .gitlab-ci.yml but ignored

See this part of the log

I left some comments in the MR.

As for the ".gitlab-ci.yml" being ignored by the tool. It seems to be the default. I did: npx eslint --debug .gitlab-ci.yml with an empty .eslintrc.js file and I still got 0:0 warning File ignored by default. Use a negated ignore pattern (like "--ignore-pattern '!<relative/path/to/filename>'") to override.

https://github.com/eslint/eslint/issues/12938
https://github.com/eslint/eslint/issues/16265#issuecomment-1344037580

Thanks for the investigation about eslint. I've just tested, and we can "unignore" our .gitlab-ci.yml, so I will open a separate issue for that.

Good comments on the MR. I had already done some more changes, so will push them now, then make the improvements you suggest.

Regarding the actual keyword order, I have not found any other scripts which are doing this work, and also not found any other discussions or best practice other than the link in #2. Tim's reasoning seams good to me, and in many cases we are already following it, so I'm going to add comments about the groups of keywords, then I could start fixing to see how much of a change it will be. After review, we can easily change the order to refine our decisions.

Agree, let's see how much moving around it is and take it from there. Once we do the first big "moving", then keeping it consistent will be easier.

I have pushed changes to fix our .gitlab-ci.yml for keyword order. Also I have added indentation in the messages, and it looks much more readable.
https://git.drupalcode.org/issue/gitlab_templates-3518793/-/jobs/5034970

The changes to run-local-checks.sh may seem unrelated, but I wanted a way to run it with both the debug and the fix arguments at the same time. So to allow any way round, and also cover 'fix' and '--fix' I added pattern-matching on the word against $1 and $2

I have added the line-number feature as requested, and also made some improvements to streamline the output, as discussed. This is ready for review, and I have not fixed the two main files yet, so that you can see the output.

But I will now fix them, so that we can see the changes.

It looks great so far. I'd just change the comment mentioned in the MR slightly to make it more obvious about why that part is there, but other than that (which is really minor) we can start changing the "main" and "main-d7" file. Setting to NW based on that.

Regarding the actual order of keywords, the order used by Tin Plavec is good, and I would like to highlight two points:

1. He has stage, needs, rules and when we use this order we get a total of 41 jobs with at least one keyword in the wrong place. But we have mostly used stage, rules, needs. If we switch to this order we only get 27 jobs with an incorrect order. So I propose we use this, as it more closely follows our practice so far, and is still a good logical grouping of those keywords.
2. A major change in the new order is having allow_failure and artifacts after the script, whereas we have generally (but not always) had these before the scripts. There is a good logical reasoning for having them after, which came from Tin's blog, and which I have replicated in the comments in the code here. I will push some of these changes, then we can take a view on whether we like the look of it

I'm happy with 1 and 2 and also not surprised at having that many as we've been creating new jobs without thinking of the order of the lines, so this issue is a great step into standarising that. We don't need to go 1:1 with Tin's suggestion (as you propose in 1), we just need to setup a logical standard and then follow it.

and which I have replicated in the comments in the code here.

I've seen the commends on the code and they are great!!

From my point of view, the script that checks the order is in a great position, so we just need to move more elements in the two main files.

I have fixed the build and validate stages and pushed the changes. When moving the interruptible keyword I realised that we set the tdefault to true in the default: section, so there is no need to set this to true again in the jobs.

I have also now just fixed the remainder of the jobs - it wss a really quick and easy task, given the line number addition, and the clear output. Many of the fixes only involved moving one item.

I'm ... not surprised at having that many as we've been creating new jobs without thinking of the order

I added a count of the jobs checked and it is 97, so with only 27 jobs to fix it meant we had more than two thirds with no changes :-)

Attached is a screenshot of run-local-checks when all is clean.

This is great so far! Give that it's an internal check and mostly just moving code, we can merge whenever you are happy with it (I am). If working on the second level is going to be quick and easy, we can do it here, otherwise I'm happy to merge this as is, and then do the second level in a different MR.

I'll let you decide what you think is best here.

Thanks. I'd be happy to merge this now, then look at the second-level changes in a new MR. I know what is needed, but not sure the complication in the script is worth it. But it may be, so needs investigation. I have pushed one final commit, to address one @todo renaming a variable and removing one unused one. If the downstream pipelines all pass then I'm happy for this to be merged now, as it does touch a lot of code, therefore doing it while other MRs are quiet makes sense.

This is merged now. Big big thanks!

Feel free to reopen and reuse this issue for further MRs or to do it in a follow up. I'm happy either way.

Thanks for merging. I think I will do this in the same issue, so I have created a new branch. May not start the work just yet.