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

Replace superlinter with smaller linters #150

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

Conversation

Alex-deVis
Copy link

Prerequisite Checklist

  • Read the contribution guidelines regarding submitting new changes to the project;
  • Tested your changes against relevant architectures and platforms;
  • Updated relevant documentation (if needed).

Description of changes

Superlinter is currently the biggest bottleneck in merging contributions. This is because of its lengthy output and the cumbersome setup required to run it locally.
In reality, we only need linters for C/C++, markdown, python, and bash and a way to run them locally in under 5 minutes.

Note: This PR seems lengthy because it adds the unchanged custom markdown rules from OEH actions.

Superlinter is way to masive for our usecase and we should opt for
smaller linters instead. Based on experience, we require: checkpatch,
markdownlint, cpplint, black, and spellcheck.

Signed-off-by: Alex Apostolescu <[email protected]>
Add markdownlint linter both as an action and as a Makefile rule.

Signed-off-by: Alex Apostolescu <[email protected]>
Add cpplint linter both as an action and as a Makefile rule. I was
unable to find a docker image to run cpplint so for the moment it runs
locally.

Signed-off-by: Alex Apostolescu <[email protected]>
Add black python linter both as an action and as a Makefile rule.

Signed-off-by: Alex Apostolescu <[email protected]>
Add shellcheck linter both as an action and as a Makefile rule.
Fail on severity greater than 'warning' to avoid refactoring current
scripts.

Signed-off-by: Alex Apostolescu <[email protected]>
@Alex-deVis Alex-deVis self-assigned this Dec 14, 2024
@teodutu
Copy link

teodutu commented Dec 14, 2024

I don't like this. We also have Python scripts and a few Go and Java programs. We use one solution to rule them all. Moreover, we already use some global existing configs from this repo [1] which is one of the few things we've done well with this repo's layout. Your PR duplicates a good portion of those.

The bottleneck isn't the super-linter; it's the students' lack of experience using CI/CDs and actually looking at what's failing. Otherwise, you can easily search for the errors shown at the end by super-linter in that massive output and find their source relatively easily.

@razvand what do you think?

[1] https://github.com/open-education-hub/actions/tree/main/super-linter

@Alex-deVis
Copy link
Author

We also have Python scripts and a few Go and Java programs.

black is a python linter and is the one used by superlinter as the others were manually disabled. Nobody took the time to configure Go and Java linters so it would be not much extra work to add those.

Moreover, we already use some global existing configs from this repo [1]

Which hides the configuration for linters and provides no way of running it locally. Using a single linter provides no benefit besides writing less in actions.yaml.

it's the students' lack of experience using CI/CDs

I'd also be intimidated if you'd hit me with 8k lines of logs even if nothing failed.

@github-actions github-actions bot added area/assignment Update to homework assignments area/infra Update to infrastructure / scripts area/tasks Update to tasks topic/compute Related to "Compute" chapter topic/software-stack Related to "Software Stack" chapter kind/improve Improve / Update existing content / item kind/new New content / item labels Dec 14, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/assignment Update to homework assignments area/infra Update to infrastructure / scripts area/tasks Update to tasks kind/improve Improve / Update existing content / item kind/new New content / item topic/compute Related to "Compute" chapter topic/software-stack Related to "Software Stack" chapter
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants