-
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
ci: add autocorrect-lint workflow for zh-CN #4191
Conversation
Preview URLs Flaws (14)URL:
URL:
(comment last updated: 2023-01-17 14:24:31) |
@huacnlee if i understand #3490 (comment) correctly, we should limit the lint to run only on the changed file and only if they are under /zh-cn folder, did the patch include these restrictions? |
I think zh-tw and jp, kr need to discussion, AutoCorrect can support them, but the MDN Core team needs to consider to enable it. |
7a1296a
to
5bb2391
Compare
5bb2391
to
c17e7cb
Compare
steps: | ||
- uses: actions/checkout@v2 | ||
with: | ||
fetch-depth: "30" |
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.
This means fetch recent 30 commits for get changed files.
In usually, a PR request commits times low that this number.
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.
Hi @huacnlee, we just find a better solution to get changes files, could you please reflect the changes made in #10390. If you need any help from me, feel free to @
me.
translated-content/.github/workflows/pr-check_markdownlint.yml
Lines 18 to 30 in abc2a5d
env: | |
GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} | |
BASE_SHA: ${{ github.event.pull_request.base.sha }} | |
HEAD_SHA: ${{ github.event.pull_request.head.sha }} | |
run: | | |
# Use the GitHub API to get the list of changed files | |
# documenation: https://docs.github.com/rest/commits/commits#compare-two-commits | |
DIFF_DOCUMENTS=$(gh api repos/{owner}/{repo}/compare/${{ env.BASE_SHA }}...${{ env.HEAD_SHA }} \ | |
--paginate \ | |
--jq '.files | .[] | select(.status|IN("added", "modified", "renamed", "copied", "changed")) | .filename') | |
# filter out files that are not markdown | |
DIFF_DOCUMENTS=$(echo "${DIFF_DOCUMENTS}" | egrep -i "^files/.*\.md$" | xargs) | |
echo "DIFF_DOCUMENTS=${DIFF_DOCUMENTS}" >> $GITHUB_ENV |
See also:
c17e7cb
to
057ebf2
Compare
Hi @irvin @SphinxKnight, this Pull Request still waiting review to merge. I just updated AutoCorrect to newest version. For now, the CI script for detect changed files for linting looks works correct. All things done. |
bcfa74c
to
f7f76ff
Compare
Hi @bsmth would you like to have a look on this? |
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 don't have objections to adding this so I'm leaving a +1
. Maybe @caugner would like to have a look? 👀
See my comment about sharing the duplication in the shared workflows repo for future improvement. 👍🏻
pinging @LeoMcA for another set of eyes 👀 |
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, but let's add some more comments to provide context.
Hi @huacnlee, do you have time to take a look :) |
Adding newline suggestion Co-authored-by: Claas Augner <[email protected]>
What can I need to do? Add the doc link? |
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 a lot, I see all of @caugner's comments are addressed and I think we should merge.
Thanks everyone 👍🏻
I'm coming here after seeing CI failures in a few PRs I recently submitted to Prettier-ify the Simplified Chinese locale. I think that this was merged a little prematurely. While the CI test is great, there's no easy way to apply the changes it is requesting. We have not added the package as a dependency, so we don't have easy access to the commands that allow us to locally test and fix reported issues. I should not have to rely on GitHub Actions to report issues in my commits; I should be able to run local, preconfigured commands to find all issues. I will submit a follow-up PR to add the local commands and integrate this into our local workflows, once I've worked out the issues. In the meantime, I've disabled the workflow, and will re-enable it once we have set up a locally runnable command. |
Yes. It's a better idea to add pre-commit hook to correct the format before committing. And we may need to fix the format of all zh-CN documents again. |
Continue for #3490, revert AutoCorrect again, improve CI for only check changed files.