Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Correct spelling of containername argument. #1571

Merged
merged 1 commit into from
Oct 5, 2022
Merged

Conversation

Kurt-von-Laven
Copy link
Collaborator

Fixes #1570.

Proposed Changes

  1. Spell the new mega-linter-runner argument container-name for consistency with the package name, mega-linter-runner, and to spare users from needing to add containername as a CSpell exception.
  2. Remove the CSpell exception for containername to prove that no references were missed and as a regression test.
  3. Make minor formatting improvements to runner.js for consistency with Allow removal of docker container when done #1563 based on my code review feedback on fixed a bug that caused --containername to not work #1561.

Readiness Checklist

Author/Contributor

  • If documentation is needed for this change, has that been included in this pull request

Reviewing Maintainer

  • Label as breaking if this is a large fundamental change
  • Label as either automation, bug, documentation, enhancement, infrastructure, or performance

@nvuillam
Copy link
Member

nvuillam commented Jun 28, 2022

🦙 MegaLinter status: ⚠️ WARNING

Descriptor Linter Files Fixed Errors Elapsed time
✅ BASH bash-exec 7 0 0.04s
✅ BASH shellcheck 7 0 0.3s
✅ BASH shfmt 7 0 0 0.28s
✅ COPYPASTE jscpd yes no 2.47s
✅ DOCKERFILE hadolint 105 0 8.05s
✅ JSON eslint-plugin-jsonc 21 0 0 2.4s
✅ JSON jsonlint 19 0 0.25s
✅ JSON v8r 21 0 18.26s
⚠️ MARKDOWN markdownlint 294 0 5 6.53s
✅ MARKDOWN markdown-link-check 294 0 6.43s
✅ MARKDOWN markdown-table-formatter 294 0 0 8.03s
✅ OPENAPI spectral 1 0 0.91s
⚠️ PYTHON bandit 170 55 2.87s
✅ PYTHON black 170 0 0 5.28s
✅ PYTHON flake8 170 0 2.41s
✅ PYTHON isort 170 0 0 0.69s
✅ PYTHON mypy 170 0 7.8s
✅ PYTHON pylint 170 0 13.22s
⚠️ PYTHON pyright 170 274 18.94s
✅ REPOSITORY checkov yes no 32.61s
✅ REPOSITORY git_diff yes no 0.26s
✅ REPOSITORY secretlint yes no 10.58s
✅ REPOSITORY trivy yes no 24.29s
✅ SPELL cspell 706 0 21.28s
✅ SPELL misspell 533 0 0 0.85s
✅ XML xmllint 3 0 0.0s
✅ YAML prettier 80 0 0 3.22s
✅ YAML v8r 22 0 42.24s
✅ YAML yamllint 81 0 1.43s

See errors details in artifact MegaLinter reports on CI Job page

MegaLinter is graciously provided by OX Security

@nvuillam
Copy link
Member

nvuillam commented Jun 28, 2022

🦙 MegaLinter status: ⚠️ WARNING

Descriptor Linter Files Fixed Errors Elapsed time
✅ BASH bash-exec 7 0 0.04s
✅ BASH shellcheck 7 0 0.27s
✅ BASH shfmt 7 0 0 0.03s
✅ COPYPASTE jscpd yes no 1.96s
✅ DOCKERFILE hadolint 105 0 8.42s
✅ JSON eslint-plugin-jsonc 21 0 0 1.81s
✅ JSON jsonlint 19 0 0.26s
✅ JSON v8r 21 0 15.77s
⚠️ MARKDOWN markdownlint 294 2 5 4.95s
✅ MARKDOWN markdown-link-check 294 0 4.96s
✅ MARKDOWN markdown-table-formatter 294 2 0 5.38s
✅ OPENAPI spectral 1 0 0.73s
⚠️ PYTHON bandit 170 55 2.45s
✅ PYTHON black 170 0 0 3.58s
✅ PYTHON flake8 170 0 2.18s
✅ PYTHON isort 170 0 0 0.4s
✅ PYTHON mypy 170 0 6.98s
✅ PYTHON pylint 170 0 10.73s
⚠️ PYTHON pyright 170 272 16.28s
✅ REPOSITORY checkov yes no 25.96s
⚠️ REPOSITORY devskim yes 59 1.15s
✅ REPOSITORY dustilock yes no 2.36s
✅ REPOSITORY git_diff yes no 0.04s
✅ REPOSITORY secretlint yes no 3.35s
✅ REPOSITORY syft yes no 2.0s
✅ REPOSITORY trivy yes no 17.43s
✅ SPELL cspell 706 0 16.04s
✅ SPELL misspell 533 2 0 0.41s
✅ XML xmllint 3 0 0.0s
✅ YAML prettier 80 0 0 2.57s
✅ YAML v8r 22 0 34.94s
✅ YAML yamllint 81 0 1.33s

See errors details in artifact MegaLinter reports on CI Job page

You could have the same capabilities but better runtime performances if you use a MegaLinter flavor:

MegaLinter is graciously provided by OX Security

@nvuillam
Copy link
Member

@Kurt-von-Laven this would be a breaking change for people using containername argument :/

@Kurt-von-Laven
Copy link
Collaborator Author

Is it worth doing in v7? I think the current unconventional spelling may cause some frustration for users who aren't yet using it. I understandably got push back from the CSpell maintainer when I proposed adding containername to the software-terms dictionary since he couldn't find much evidence of other projects that use this spelling.

@nvuillam
Copy link
Member

Let's have a stable v6 before thinking about v7 haha ^^
But we can write that somwhere for someday ^^

@github-actions
Copy link
Contributor

This pull request has been automatically marked as stale because it has not had recent activity.
It will be closed in 14 days if no further activity occurs.
Thank you for your contributions.

If you think this pull request should stay open, please remove the O: stale 🤖 label or comment on the pull request.

@github-actions github-actions bot added the O: stale 🤖 This issue or pull request is stale, it will be closed if there is no activity label Aug 15, 2022
@Kurt-von-Laven Kurt-von-Laven added nostale This issue or pull request is not stale, keep it open and removed O: stale 🤖 This issue or pull request is stale, it will be closed if there is no activity labels Aug 20, 2022
@github-actions
Copy link
Contributor

This pull request has been automatically marked as stale because it has not had recent activity.
It will be closed in 14 days if no further activity occurs.
Thank you for your contributions.

If you think this pull request should stay open, please remove the O: stale 🤖 label or comment on the pull request.

@github-actions github-actions bot added the O: stale 🤖 This issue or pull request is stale, it will be closed if there is no activity label Sep 20, 2022
@Kurt-von-Laven Kurt-von-Laven removed the O: stale 🤖 This issue or pull request is stale, it will be closed if there is no activity label Sep 20, 2022
@Kurt-von-Laven Kurt-von-Laven force-pushed the container-name branch 2 times, most recently from 91321bc to 7331c31 Compare September 21, 2022 21:12
@codecov-commenter
Copy link

codecov-commenter commented Sep 21, 2022

Codecov Report

Merging #1571 (8c2c728) into main (1708e04) will increase coverage by 0.02%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##             main    #1571      +/-   ##
==========================================
+ Coverage   82.68%   82.71%   +0.02%     
==========================================
  Files         157      157              
  Lines        3384     3384              
==========================================
+ Hits         2798     2799       +1     
+ Misses        586      585       -1     
Impacted Files Coverage Δ
megalinter/reporters/UpdatedSourcesReporter.py 89.74% <0.00%> (+2.56%) ⬆️

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

Spell the new mega-linter-runner argument container-name for consistency
with the package name, mega-linter-runner, and to spare users from
needing to add containername as a CSpell exception. Continue to accept
containername for backwards compatibility.
Copy link
Member

@nvuillam nvuillam left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

great :)

@nvuillam nvuillam merged commit 2a6ddcd into main Oct 5, 2022
@nvuillam nvuillam deleted the container-name branch October 5, 2022 08:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working nostale This issue or pull request is not stale, keep it open
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Rename containername mega-linter-runner argument to container-name
4 participants