-
Notifications
You must be signed in to change notification settings - Fork 8.2k
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
Add autocorrect-node
and configure that run lint before git commit.
#14773
Conversation
Hi @huacnlee. Should we also use By the way, do you have time to run a full autocorrect on zh-cn files again, and create some PRs to fix them :) After that we should also add ci test to check if all the zh-cn documents are still "good" when we update the version of |
31572b1
to
e201d58
Compare
5b511ef
to
df59f13
Compare
df59f13
to
0848038
Compare
Preview URLs (comment last updated: 2023-12-09 05:44:58) |
739532e
to
1a1ea1d
Compare
As there are security limitation on GitHub Actions. Could you merge the workflow here into pr-check-lint_content, or do the same changes as what we made in this workflow - spreading into two jobs and use |
This pull request has merge conflicts that must be resolved before it can be merged. |
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.
@huacnlee I've pushed some commits to merge autocorrect into our system. Could you have a look on this.
And I'm not sure about the ignore rules:
Co-authored-by: A1lo <[email protected]>
Co-authored-by: A1lo <[email protected]>
Co-authored-by: A1lo <[email protected]>
@yin1999 The |
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.
LGTM. Thanks @huacnlee.
ping @mdn/core-dev for the review from the development team
This pull request has merge conflicts that must be resolved before it can be merged. |
This pull request has merge conflicts that must be resolved before it can be merged. |
@@ -61,6 +61,7 @@ jobs: | |||
run: | | |||
yarn markdownlint-cli2 --fix "**/${{ matrix.lang }}/**/*.md" | |||
node ./scripts/check-url-locale.js --fix "files/${{ matrix.lang }}" | |||
ls -d 2>/dev/null "files/${{ matrix.lang }}" "docs/${{ matrix.lang }}" | xargs yarn autocorrect --fix |
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.
Using ls -d
to filter out non-existed dirs
@bsmth Sorry to bother you, this PR has been there for a long time. And most of the content changes come from myself, I think it would be better to have a double check from other people before we merge this. Do you have time to have a look on this :) |
This doesn't bother me at all, please continue to ping me on PRs where I can help review or unblock - that's what I'm here for :) I will have a look today 🙌🏻 |
# - correct: "Welcome, to read MDN Web Docs." | ||
# | ||
# More details: | ||
# https://github.com/huacnlee/autocorrect |
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 might be nice to have this documented somewhere in this repo, but not a blocker for this PR.
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.
No objections from me apart from adding some docs / linking to https://github.com/huacnlee/autocorrect somewhere. A README.md in https://github.com/mdn/translated-content/tree/main/.github/workflows? We can do in a follow-up.
It would be great to have a sign off from dev on this, also 👍🏻 |
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.
LGTM, only affects zh-CN.
Improves "copywriting, correct spaces, words, and punctuations". (Source: https://github.com/huacnlee/autocorrect) Only enabled for zh-CN for now. Also runs before git commits. Co-authored-by: A1lo <[email protected]>
This will just be checking the changed files before git commit.
The
autocorrect
command provided byautocorrect-node
NPM, there are no other binary need to install.It also will follow the
.autocorrectignore
and.autocorrectrc
rules.Ref: #4191
@yin1999 @queengooborg