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

Configurable linter commands #4482

Open
wants to merge 6 commits into
base: main
Choose a base branch
from
Open

Conversation

bdovaz
Copy link
Collaborator

@bdovaz bdovaz commented Jan 10, 2025

Resolves #4015

Let's see what you think @nvuillam

Copy link
Contributor

github-actions bot commented Jan 10, 2025

🦙 MegaLinter status: ⚠️ WARNING

Descriptor Linter Files Fixed Errors Elapsed time
✅ API spectral 1 0 1.74s
⚠️ BASH bash-exec 6 1 0.03s
✅ BASH shellcheck 6 0 0.21s
✅ BASH shfmt 6 0 0 0.77s
✅ COPYPASTE jscpd yes no 2.81s
✅ DOCKERFILE hadolint 129 0 23.63s
✅ JSON jsonlint 20 0 0.2s
✅ JSON v8r 22 0 13.71s
⚠️ MARKDOWN markdownlint 267 0 303 23.54s
✅ MARKDOWN markdown-table-formatter 267 0 0 159.91s
⚠️ PYTHON bandit 215 66 2.96s
✅ PYTHON black 215 0 0 4.4s
✅ PYTHON flake8 215 0 1.75s
✅ PYTHON isort 215 0 0 1.21s
✅ PYTHON mypy 215 0 16.64s
✅ PYTHON pylint 215 0 33.21s
✅ PYTHON ruff 215 0 0 0.8s
✅ REPOSITORY checkov yes no 35.43s
✅ REPOSITORY git_diff yes no 0.47s
⚠️ REPOSITORY grype yes 26 12.71s
✅ REPOSITORY secretlint yes no 11.55s
✅ REPOSITORY trivy yes no 14.48s
✅ REPOSITORY trivy-sbom yes no 0.38s
⚠️ REPOSITORY trufflehog yes 1 54.61s
✅ SPELL cspell 718 0 12.65s
⚠️ SPELL lychee 349 17 9.23s
✅ XML xmllint 3 0 0 0.93s
✅ YAML prettier 160 0 0 3.4s
✅ YAML v8r 102 0 29.45s
✅ YAML yamllint 161 0 3.01s

See detailed report in MegaLinter reports

MegaLinter is graciously provided by OX Security

@nvuillam
Copy link
Member

@bdovaz please can you explain what you do ?

It's not that obvious when quickly reading the PR updated files :/

cc @echoix

@echoix
Copy link
Collaborator

echoix commented Jan 12, 2025

I didn't really understood the issue nor this fix..

@bdovaz
Copy link
Collaborator Author

bdovaz commented Jan 12, 2025

  • The pre/post commands at linter level have a new bool option called parallel with default value to true so that it is not a breaking change.
  • The aforementioned option makes these commands that have the value as false to be executed before/after executing the linters in parallel.
  • Inside the function that makes the linters to be called in parallel the commands with the value to true are executed.
  • If at global level the value of parallel is set to false which makes the linters to be executed sequentially, they ignore the value of the parallel command of the linter and all of the commands are executed.

cc @echoix

@nvuillam
Copy link
Member

@bdovaz i get it, it seems nice :)

Please can you

@bdovaz
Copy link
Collaborator Author

bdovaz commented Jan 18, 2025

@nvuillam done! Check if I haven't screwed up by refactoring the command and having to invert the bool.

Thanks!

cc @echoix

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.

Plz check comments :)
As it is impacting, I'd also like a double-check from @echoix :)

CHANGELOG.md Outdated Show resolved Hide resolved
self.process_linters_parallel(active_linters, linters_do_fixes)

Copy link
Member

Choose a reason for hiding this comment

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

Do you see a use case where we need it in post commands ?
if not you can remove this part

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I can't think of it right now but “it's free” to have the same functionality in both pre and post, isn't it? Better to have it than not to have it.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

What do you think @echoix?

if linter.post_commands is not None:
filtered_commands: list = []
Copy link
Member

Choose a reason for hiding this comment

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

If not needed for post_commands, plz remove this part

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Read the comment above

README.md Show resolved Hide resolved
CHANGELOG.md Outdated Show resolved Hide resolved
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Error installing prettier plugin with pre commands
3 participants