mermaid-cli
mermaid-cli copied to clipboard
Lint using `standard`
:bookmark_tabs: Summary
Lints this project using standard
.
I've added running standard
to the package.json#scripts#test
(in https://github.com/mermaid-js/mermaid-cli/commit/87c83014a15f7cf7c2ab430cd1dc6b4fa07fefc6), since that's what it says in CONTRIBUTING.md
:
https://github.com/mermaid-js/mermaid-cli/blob/6fa464418488ead911faa2537ed4e2c33fc267f1/CONTRIBUTING.md?plain=1#L23
I've also added a GitHub Actions CI action that runs yarn test
automatically for future PRs in https://github.com/mermaid-js/mermaid-cli/commit/34017714315dd1577e16610e1669aab7e7b0d228.
Finally, I've fixed any errors that were NOT automatically fixed by standard --fix
in https://github.com/mermaid-js/mermaid-cli/commit/e77b3c92a0b59d5cc4adb975441925c75081af40.
This is to reduce the git diff
and make this PR easier to review/less likely to cause merge conflicts. All other linting errors should be fixed by running standard --fix
right before/after merging (example: https://github.com/aloisklink/mermaid-cli/commit/0b936615694aa3441e6ba2b35c4ba0e78e249aa6).
:straight_ruler: Design Decisions
To simplify the .github/workflows/lint.yml
file, I've moved the prepublishOnly
script into a prepare
and prepack
script, as npm
/yarn
will call those scripts automatically, when needed (https://github.com/mermaid-js/mermaid-cli/commit/2a0965e1aa06a84739a8de44e33128110602185f).
npm run prepare
is recommended for building NPM packages, as it's called automatically before npm pack
, npm publish
, and npm install .
(local) or npm install git+https://
(git dependency).
yarn run prepack
is recommended for building yarn packages, as it's called before yarn pack
, yarn publish
, and automatically when installing from "raw" sources, like a git dependency.
I've also modified all the *.sh
scripts to have a !/bin/sh
shebang, since the Alpine Linux Docker image doesn't actually support bash. Only run-tests.sh
required a very minor change to be /bin/sh
compliant (https://github.com/mermaid-js/mermaid-cli/commit/b43c86c3b0ee7def0f017adde157dc33077b25b1).
:clipboard: Tasks
Make sure you
- [x] :book: have read the contribution guidelines
- Auto-fixable linting issues have not been fixed, to make the
git diff
easier to review. Please runyarn run lint-fix
right before/after merging.
- Auto-fixable linting issues have not been fixed, to make the
- [x] :computer: have added unit/e2e tests (if appropriate)
- [x] :bookmark: targeted
master
branch
Thank you for another high quality PR. Please, resolve the conflicts and have a look at my comment.
I've rebased my commits onto master
to fix the conflict. (git merge didn't seem to do anything, since the project uses rebasing).
I can't find your comment. That may be my fault or GitHub UI's fault: it might have disappeared when I resolved the git conflicts.
I haven't yet run yarn standard --fix
(fixes minor style errors automatically), to make this PR smaller and easier to review.
Because of that, the linting CI is failing. Let me know if/when you want me to add that commit, or if you're happy to add it.
And thanks to you too for maintaining this super awesome tool :smile:
I am busy at the office now. I will get back to you in a couple of weeks. I am sorry for the delay :) I will tag in the comment. 😄
@aloisklink can you rebase your branch? I see a great value in this PR 😄
@aloisklink can you rebase your branch? I see a great value in this PR :smile:
:rocket:
Rebased onto https://github.com/mermaid-js/mermaid-cli/commit/9fa8d397633d0265c22c63a345e834ec73d812cf (current master
)
I've slightly modified this PR too, since some of my other PRs have made part of this PR outdated.
CI is failing because I haven't run yarn standard --fix
yet, to make review easier.
I can do it before merging, or you can do it after merging.