feat: Skip CI on docs-only changes #705
Conversation
|
Hi @ongdisheng, I think using an ignore-based approach would be cleaner than the include logic. |
|
Hi @GOODBOY008, thank you for working on this optimization! I do agree that the
That's about 20+ files that would still run Java CI builds unnecessarily while the include approach ensures CI only runs for actual code and build changes. I think both approaches have their own trade-offs. Feel free to let me know what do you think? |
|
@ongdisheng The scenarios you listed will still run the CI pipeline and it's very necessary with my implement.Just like action file, if add new one you need to trigger as your implement you will modify the action conditions? It's too complexity. |
|
@ongdisheng For example, if someone accidentally modifies the mvnw file incorrectly, the CI won’t run and the change could be merged into main, causing CI failures later. Because the PR CI is skipped, this kind of issue cannot be detected in time. |
| - '.github/workflows/codeql-scan.yml' | ||
| - '**/pom.xml' | ||
| - 'fesod/**' | ||
| - 'fesod-*/**' | ||
| - '!**.md' |
There was a problem hiding this comment.
| - '.github/workflows/codeql-scan.yml' | |
| - '**/pom.xml' | |
| - 'fesod/**' | |
| - 'fesod-*/**' | |
| - '!**.md' | |
| - "**.java" |
Only java file to trigger ci.
There was a problem hiding this comment.
Hi @GOODBOY008, Thanks for the suggestion! Just wondering, would "**.java" actually cover everything we need here? From what I can see, it would only match Java source files, but we also have test resources like Excel and CSV files in src/test/resources/ that our tests depend on. If someone updates those test files, we probably want CI to run and make sure the tests still pass. Also, "**.java" wouldn't catch changes to pom.xml or the CI workflow itself and it would possibly still have the same mvnw issue you pointed out earlier. Just wondering if we just add mvnw on top of the existing list to cover the Maven wrapper case, would this actually be better?
There was a problem hiding this comment.
Thank you all for your discussion.
IMO, I think this setting should simplify the scene rather than make it more complicated. If in a PR only the resources file or the mvnw file is modified separately, this PR may be directly rejected during the review process.
| - '.github/workflows/codeql-scan.yml' | ||
| - '**/pom.xml' | ||
| - 'fesod/**' | ||
| - 'fesod-*/**' | ||
| - '!**.md' |
|
I recommend seeking consensus. Both PRs share the same underlying premise, though they approach it differently. We welcome both proposals. At present, I believe the |
|
Hi @delei, Thank you for the feedback. I understand you prefer the paths-ignore approach from #711. I'm happy to close my PR as what matters most is improving the CI efficiency for the project, regardless of whose implementation we use. |
Related: #703
Purpose of the pull request
This PR optimizes CI workflows to skip unnecessary Java tests when only documentation files are changed.
What's changed?
This PR added paths filtering to
ci.ymlandcodeql-scan.ymlso they only run when Java code or Maven configs change.Checklist