-
Notifications
You must be signed in to change notification settings - Fork 1.3k
feat(ci): only markdown action to skip some CI workflows #13118
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
feat(ci): only markdown action to skip some CI workflows #13118
Conversation
@kamuik16 you've got an outdated filecoin-ffi here - I think you branched just at the wrong moment and now have the wrong ffi; rebase on to master and make the ffi change go away and 🤞 your tests should pass |
…/only-markdown-lint
e232ee3
to
3c50c54
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.
Pull Request Overview
This PR adds a composite GitHub Action to detect when a PR includes only Markdown file changes and integrates a planner job in several workflows to skip heavy CI steps for docs-only PRs.
- Added a new composite action for detecting Markdown-only changes.
- Integrated a planner job into the stale, docker, check, and build workflows to conditionally skip jobs.
- Updated workflow dependencies to run jobs only when non-doc changes are present.
Reviewed Changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated no comments.
Show a summary per file
File | Description |
---|---|
.github/workflows/stale.yml | Added a planner job and conditional trigger in the stale workflow. |
.github/workflows/docker.yml | Added a planner job and corresponding if condition in the docker workflow. |
.github/workflows/check.yml | Added a planner job and updated job dependencies in the check workflow. |
.github/workflows/build.yml | Added a planner job and conditional step to skip build when docs-only. |
.github/actions/only-markdown/action.yml | Added a composite action to determine if only Markdown files changed. |
Comments suppressed due to low confidence (1)
.github/workflows/stale.yml:15
- [nitpick] Consider aligning the output variable name between the planner job (currently 'only_docs') and the composite action output ('only_markdown_changes') to improve clarity and consistency.
only_docs: ${{ steps.check.outputs.only_markdown_changes }}
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'll leave to others to give approval. I wish we didn't need to make checks in every job of of a workflow, but per #13069 I didn't see any way to accomplish this, so your approach seems right.
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.
Yeah, this is the best way to achieve this under the constraints.
For this, this PR itself can be an example (where non .md files changes and build/tests were ran), but I need to think some other way to test only for .md changes. |
If this isn't easy, I wouldn't worry about this. In the worst case, it's not working as desired for markdown only changes and we need to do followups. |
I have tested the commands locally, and it works perfectly fine, and as you said, worst case, the build and test runs, if that happens, which I'm pretty sure won't, I'll fix it. |
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.
Approving so this doesn't go more stale and because I think the potential harm here is low.
I'm going to merge this PR since there are fixes that were already in place to fix the failing tests. Given this PR is coming from a fork, I can't directly update that fork with the fixes. Rather than have more back and forth, I think this is safe to just merge. |
)" This reverts commit e53768c.
Related Issues
Closes #13069
Proposed Changes
Additional Info
Checklist
Before you mark the PR ready for review, please make sure that: