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.

Sign up for GitHub

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

Jump to bottom

Improve code quality by using standard tools to format and analyse for PRs #429

Open
thomasmerz opened this issue Mar 28, 2022 · 4 comments · May be fixed by #435
Open

Improve code quality by using standard tools to format and analyse for PRs #429

thomasmerz opened this issue Mar 28, 2022 · 4 comments · May be fixed by #435

Comments

@thomasmerz
Copy link

Your spectre-meltdown-checker.sh is already shellcheck'ed, but you don't have a GitHub Action to enforce this before a Pull Requests can be merged.

And I have seen, that you are doing some "low-level" / very simple format-checking. I suggest to use shfmt which formats shell programs. This can also be enforced by a GitHub Action before a Pull Request can be merged.

If you @speed47 agree I will make a PR with my suggestion regarding a nice GitHub Action for every push on this script and on every PR.
@speed47
Copy link
Owner

speed47 commented Mar 29, 2022

Yes, that would be a good idea! You'll probably have to find out which shfmt parameters work best to avoid rewriting the entire script (I don't want to lose all the git blame usefulness), and mainly ensure that future PR just respect the current coding style.

That's also why I always left tabs (I usually use spaces!), because I noticed that too late and didn't want to rewrite the entire git history :)

@thomasmerz
Copy link
Author

Challenge accepted 👍🏻
This might take some time, because it's not in a hurry and I'll be looking for some "creative time"…

@thomasmerz thomasmerz linked a pull request Mar 30, 2022 that will close this issue
Open
@thomasmerz
Copy link
Author

@speed47 , can you please review my PR #435 and merge if you are satisfied? Thanks a lot 👍

@thomasmerz
Copy link
Author

Ping @speed47

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Issue 429: analyse/lint and format script thomasmerz/spectre-meltdown-checker
2 participants
@speed47 @thomasmerz