-
Notifications
You must be signed in to change notification settings - Fork 1.2k
fix(github-actions): changed files output for editorcondig-checker #1180
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
Conversation
Seems that ci.yml includes a dead dependency action just to get list of changed files for the PR / push. This seems weird as the repo is all about git. Propose removing this extra dep with straightforward implemenation. Also: github runners do have `jq` preinstalled.
2ca553e
to
9d36a8f
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 for your contribution!
.github/workflows/ci.yml
Outdated
run: | | ||
{ | ||
echo "added_modified<<EOF" | ||
git diff --name-only "${BASE_SHA:-$BEFORE_SHA}...${HEAD_SHA:-GITHUB_REF}" |
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.
Would you print the diff result to the output so that we can check it if the CI doesn't work well?
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.
Hey, this can be verified by running the github actions in debug mode, then github will log the contents of the env variable. But for sure it would be easier by just printing that in the step. I will amend!
@oikarinen |
@oikarinen |
Yeah that's it! i did test the pull_request and then called it good and the push path was not covered 🥹 |
@spacewander here's the missing |
Seems that ci.yml includes a dead dependency action just to get list of changed files for the PR / push. This seems weird as the repo is all about git. Propose removing this extra dep with straightforward implementation that supports force push for the PR branches.
Also:
jq
preinstalled, so that was redundant and now no longer needed.