Apply pr2957 to mega-linter-runner/.../mega-linter.yml #3033
Conversation
Signed-off-by: Ross Smith II <[email protected]>
rasa
requested review from
nvuillam,
echoix,
bdovaz and
Kurt-von-Laven
as code owners
|
From what I recall, the skip fix wasn't a recommended or documented feature of mega-linter. It was an old quick and dirty way @nvuillam used alone when he wanted to not have MegaLinter run (and fail) on his commits. It isn't something we would have for all users, it was his customization. Edit: I really looked and I didn't see much usage in here, but it was publicly written in the changelog and was in the template generator. So, it was in the documentation, but not really documented. So... yah, it would be breaking to remove it, but it is really only a GitHub Actions feature that can be customized, not really MegaLinter itself. |
|
I see that this PR would cause a couple conflicts to #3032 if it was merged before, and vice-versa. |
| ) && | ||
| !contains(github.event.head_commit.message, 'skip fix') |
There was a problem hiding this comment.
| ) && | ||
| !contains(github.event.head_commit.message, 'skip fix') | ||
|
|
There was a problem hiding this comment.
| ) && | ||
| !contains(github.event.head_commit.message, 'skip fix') |
There was a problem hiding this comment.
There was a problem hiding this comment.
I would prefer the || solution, but it's more important that we apply this the same logic across all five (?) instances of mega-linter.yml.
There was a problem hiding this comment.
... but it's more important that we apply this logic across all five (?) instances of mega-linter.yml.
Seems weird like that. We have one that is our own for dogfooding (the python@beta), another one for the second project (the node mega-linter-runner) again for dogfooding, then there is the one that is for the template for users, and all the duplicatas built into the documentation... I expect that these update from their sources when running ./build.sh, except if I'm wrong (and we should fix it).
| # Trigger mega-linter at every push. Action will also be visible from Pull | ||
| # Requests to main |
There was a problem hiding this comment.
| @@ -41,16 +41,17 @@ jobs: | |||
| runs-on: ubuntu-latest | |||
|
|
|||
| # Give the default GITHUB_TOKEN write permission to commit and push, comment | |||
| # issues, and post new Pull Requests; remove the ones you do not need | |||
| # issues & post new PR; remove the ones you do not need | |||
There was a problem hiding this comment.
|
|
||
| # Git Checkout | ||
| - name: Checkout Code | ||
| uses: actions/checkout@v3 | ||
| uses: actions/checkout@v4 |
There was a problem hiding this comment.
There was a problem hiding this comment.
@echoix one thing to consider with an upgrade to checkout@v4 is it may break workflows for people who use local linting. I'm thinking about tools like https://github.com/nektos/act - they're falling pretty far behind in keeping up with GitHub on server images. @catthehacker is the user that was maintaining those and they have left GitHub. It is possible that an upgrade to actions/checkout@v4, which, if I recall correctly upgrades the node version required, could break MegaLinter's ability to run locally via act.
It's something we need to earmark for a test (and I have with the change I proposed) prior to merging this into MegaLinter's next release.
There was a problem hiding this comment.
I think that the most important use case isn't with act, and thus, it way more important to not use a node version that is EOL since Sept 11, 2023 (node 16 used in actions/checkout@v3). Running locally with act, even if I have done it a couple of times in the past (when doing big changes in the docker building actions and multiplatform builds, iterating on GitHub was way too long), remains more of a hack than a supported (by us) use-case. Having an up to date runner image for act seems like the way to solve your interrogation.
I don't understand the meaning of your last sentence, especially "earmark for a test", and what do you expect before a next release (that should be due soon IMO, maybe wait a little after we settled and merged the big template changes and other link changes PRs).
There was a problem hiding this comment.
We should have pretty up-to-date nodejs on images.
There was a problem hiding this comment.
We should have pretty up-to-date nodejs on images.
I wasn't able to run checkout@v4 on the latest images as Node 20 wasn't available, IIRC.
| uses: <%= GITHUB_ACTION_NAME %>@<%= GITHUB_ACTION_VERSION %> | ||
|
|
||
| id: ml | ||
|
|
||
| # All available variables are described in documentation | ||
| # https://megalinter.io/latest/config-file/ | ||
| # https://megalinter.io/configuration/ |
There was a problem hiding this comment.
The original would be the correct link, #2986
I just caught up on this thread - @rasa I think we could definitely get your changes incorporated. #3032 is primarily a formatting change, so if you make some proposed changes there, I can pretty easily accept them and ensure your credit is maintained. Let me know if you have any questions or concerns. |
@andrewvaughan Don't need credit. Whatever's easiest. If you want to merge in #3032 first, and have me rebase this PR after, that works. Or vice versa. |
I think we're going to finalize that issue today - keep an eye on it for a rebase. |
|
This pull request has been automatically marked as stale because it has not had recent activity. If you think this pull request should stay open, please remove the |
github-actions
bot
added
the
O: stale 🤖
github-actions
bot
removed
the
O: stale 🤖
|
Happy to let this close, if that’s the consensus. No worries. |
|
Nah, I love this PR, I'm jsut afraid of regressions ^^ Has it been tested on multiple repos ? :) |
|
If I recall correctly, wasn't the most of the changes of this PR superseded by #3032, that completely rewrites the template and docs? |
|
I love both PRs 👍 |
|
Not sure what more is needed from me here for my PR, but happy to help - just point me in the direction you need! |
|
@nvuillam @andrewvaughan @echoix Let's let this PR close, and after we merge in #3032, I can craft a PR that keeps our various1 Q: To that end, is https://github.com/oxsecurity/megalinter/blob/main/TEMPLATES/mega-linter.yml considered to be our authoritative version? Footnotes
|
|
I started something out, but my node skills are pretty rudimentary, but the process was a bit the same as how the linter helps are placed in our docs when building. We call the tool, take the output, and replace the contents between two markers. The difficulty that existed with the generator tool was that calling it interactively wasn't obvious from their docs. It kinda changed over time, and I drafted a POC that was able to call and get the contents in a temporary file. The rest is to finish to integrate into a working whole. So yes, what is used for the generator is authoritative. |
I think And there's files for the other CI platforms that could be synced in the same way into the docs. |
Gotcha. My apologies, but I didn't update this file in either #2888 or #2957, so once #3032 is merged, I will draft a PR to sync up all our various versions, if need be. Closing this PR, but its work will not be forgotten. |
Per #2957 (comment) .
This also adds
to all the variants. This line was in some of the mega-linter.yml variants, but not all of them, and the one I started from didn't have it, so I missed it.
See
5f5959d#diff-2a390738250fd23bc48f50e25700e0b2f06b62072aef23258772b1897a9b431b
My apologies for not catching this sooner.