Thank you! Just a few small things to clean up:

• I appreciate the docs update, but it's drawn the ire of the markdown link checker. Let's try just a bare relative link without the leading ./ in the path and see if it passes.
• Would you mind squashing down these changes to a single commit? We have a not-so-Git-Hub friendly workflow of requiring --no-ff merges but also there has been a push recently for very clean, atomic histories. If you can do the squash on your end first it is easier to do the merge commit at the end.
• Did you change the permissions on the scripts intentionally to work around an issue?

Thank you! Just a few small things to clean up:

• I appreciate the docs update, but it's drawn the ire of the markdown link checker. Let's try just a bare relative link without the leading ./ in the path and see if it passes.
• Would you mind squashing down these changes to a single commit? We have a not-so-Git-Hub friendly workflow of requiring --no-ff merges but also there has been a push recently for very clean, atomic histories. If you can do the squash on your end first it is easier to do the merge commit at the end.
• Did you change the permissions on the scripts intentionally to work around an issue?
  Thanks for the review

1. Fixed - it was a missing s.
2. Yea, I can, but I intentionally kept them separate. If you squash them all into a single commit, then there is no point to add a git-blame-ignore-revs file to ignore the pre-commit changes. I intentionally kept them as separate commits because they involve separate things. Given that, do you still want me to collapse them all, and drop the .git-blame-ignore-revs?
3. Yes, there is a pre-commit hook that tells you to. Those scripts have a shebang, but aren't marked with executable permissions. There are two hooks. One checks for if you have a file with executable permissions that it has a shebang if it's a script. The other hook does the opposite and checks that any script with a shebang is marked as executable. Missing one and having the other defeats the purpose.

Sounds good for 1 and 3, the script permissions were weird because they only get used in the earthly context but this is very reasonable.

For point 2 I'm fine keeping it as-is, but I'm going to defer to the author of our traceability stratergy, @ivanperez-keera.

I'm going to mark my approval but wait for Ivan's weigh-in before hitting the merge button. Sorry for the slow cadence but we appreciate your contribution!

@Ryanf55 I see several GA hooks added. In #80, you stated that "There is even support for pre-commit to auto-fix contributions and push a follow up commit."

Is that currently enabled with in this PR? If so, that is not something we want (it does not necessarily comply with our process; we had that case already in the past with a script that "fixed" our *.repos files).

As for the commit history: there's no need to squash always. Just the commit history should explain how to introduce the solution in a clean way, rather than how the developer figured out the steps. It could have one or several commits.

That being said:

• Could you make the commit message of the first commit a bit more descriptive? I had to really look to realize that it wasn't introducing two empty files (which is what the github interface says), just changing the permissions. And how does changing the executable bit fix the pre-commit errors? What errors? At that point, for the developer who looks at this without much context, what's are we doing?

• What commit is 3d3d562 listed in the .git-blame-ignore-revs file?

Also, does this entirely address #80, or only introduce the support we need to actually implement #80? If the latter, then we can accept the contribution but we need to create a new issue called "Introduce support to lint/analyze scripts in CI/CD" with the necessary description, and mention in the issue description or comments that it's related to #80. We should then edit the commit messages in this PR to mention ONLY the new issue number and remove #80.

That way, we don't scatter partial solutions for a problem around the history of the repo (it kind of defeats the point of using our development process and git merge --no-ff if branches only partially address issues). Note that our contribution policy states "All PR merges introduce a separate merge commit that closes the issue that the PR addresses". The merge commit can only close #80 if this PR fully addresses #80.

@Ryanf55 I see several GA hooks added. In #80, you stated that "There is even support for pre-commit to auto-fix contributions and push a follow up commit."

Is that currently enabled with in this PR? If so, that is not something we want (it does not necessarily comply with our process; we had that case already in the past with a script that "fixed" our *.repos files).

I did not enable autofix because we decided that already: "I do not think we should let CI make code changes"

As for the commit history: there's no need to squash always. Just the commit history should explain how to introduce the solution in a clean way, rather than how the developer figured out the steps. It could have one or several commits.

That being said:

• Could you make the commit message of the first commit a bit more descriptive? I had to really look to realize that it wasn't introducing two empty files (which is what the github interface says), just changing the permissions. And how does changing the executable bit fix the pre-commit errors? What errors? At that point, for the developer who looks at this without much context, what's are we doing?
• What commit is 3d3d562 listed in the .git-blame-ignore-revs file?

Also, does this entirely address #80, or only introduce the support we need to actually implement #80? If the latter, then we can accept the contribution but we need to create a new issue called "Introduce support to lint/analyze scripts in CI/CD" with the necessary description, and mention in the issue description or comments that it's related to #80. We should then edit the commit messages in this PR to mention ONLY the new issue number and remove #80.

That way, we don't scatter partial solutions for a problem around the history of the repo (it kind of defeats the point of using our development process and git merge --no-ff if branches only partially address issues). Note that our contribution policy states "All PR merges introduce a separate merge commit that closes the issue that the PR addresses". The merge commit can only close #80 if this PR fully addresses #80.

Got it. I'll just add the .github-ci workflow to this PR. That said, the original issue was also talking about static analysis, which is a much larger scope, and I don't think would be suitable for a single PR. I'll break out the issue to a new
"Stand up initial pre-commit configuration with CI" ticket, edit all of the commit messages to point to the new ticket, and request review once it's ready again. Also, we need to find which specific linter we want for shell - perhaps this one

From what I'm reading, adding shellcheck to the pre-commit config and setting CI config to run pre-commit would meet the original definition for #80. If you'd still rather break out the introduction of pre-commit as a separate ticket and merge the change set you have here, then add shellcheck in a separate PR we can do that too.