-
Notifications
You must be signed in to change notification settings - Fork 38
ci(linting): add remaining linting and formatting checks #231
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
ci(linting): add remaining linting and formatting checks #231
Conversation
2710cd5
to
f903b7c
Compare
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #231 +/- ##
=======================================
Coverage 27.65% 27.65%
=======================================
Files 5 5
Lines 141 141
=======================================
Hits 39 39
Misses 102 102 🚀 New features to boost your workflow:
|
f903b7c
to
4005a22
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Having introduces everything in one single commit, instead of adding each linter/formatter separately, may be acceptable -- but then we would have to alter the commit log headline message in order to produce nicer release news item for users. (Because people would not understand "add remaining linting and formatting checks" -- which remaning?) One possibility would be to write something like "add JSON, Markdown, Prettier, shmft, YAML formatting and linting" if it is not too long.
Alternatively, you can just split the commit into several dedicated atomic ones, starting with shfmt
, as yet another git rebasing exercise...
See an example of such automatically generated release notes: reanahub/reana-workflow-controller#578
@@ -0,0 +1,4 @@ | |||
# Allow prompt dollar sign in console examples without showing output | |||
MD014: false | |||
# Allow multiple headings with same content (for different releases) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Cosmetics: Let's use a blank line between these two rules?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah yes, understood. I'll see if it's difficult to split the commits.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It seems doable splitting the commits, I might not get round to all of them today
CHANGELOG.md
Outdated
# Changelog | ||
|
||
## [0.9.4](https://github.com/reanahub/reana-workflow-engine-serial/compare/0.9.3...0.9.4) (2024-11-29) | ||
|
||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I wonder whether Release Please will reintruduce empty lines when compiling next release notes. If so, then it would be better if we add another markdownlint disable statement...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah yes will do 👍
4005a22
to
6d40578
Compare
6d40578
to
29202fc
Compare
29202fc
to
acb707f
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks, slightly amended the commit log headlines for nicer release notes.
Introduce remaining file formatting checks to
run-tests.sh
and to the GitHub CI:.editorconfig
for consistent character formatting across the repoyamllint
for YAML lintingjsonlint
for JSON lintingshfmt
for shell script formattingmarkdownlint
for markdown lintingprettier
for various other formatting such as.js
filesCloses #230