-
Notifications
You must be signed in to change notification settings - Fork 15
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
install diff-filter-buildkite-plugin to only build when relevant files changed #400
base: main
Are you sure you want to change the base?
Conversation
e0bd514
to
36db0ee
Compare
I believe NEWS.md and HISTORY.md are in the docs, so I think it needs at least the docs to be built for those? |
a4fd9c6
to
9b18b56
Compare
9b18b56
to
df7d25c
Compare
- diff-filter-doc_plugin: &diff-filter-doc | ||
https://github.com/fatteneder/diff-filter-buildkite-plugin#main: | ||
name: "doc" | ||
match: | ||
- "doc/*" | ||
- "HISTORY.md" | ||
- "NEWS.md" | ||
# TODO: Need this option because BUILDKITE_PULL_REQUEST_BRANCH=main | ||
# when running tests in Julia-CI/julia-buildkite, | ||
# but for PRs against Julia/JuliaLang BUILDKITE_PULL_REQUEST_BRANCH=master | ||
target_branch: "master" |
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.
Does this mean the docs run is only run for those matches? Don't we need to cover changes to docstrings and their doctests?
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. And docstrings can be in any file. I think we pretty much always need to build the docs.
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.
Docs build should be triggered by any of these two now:
if [[ $${BUILDKITE_PLUGIN_DIFF_FILTER_TRIGGERED_BUILD} == 1 ||
$${BUILDKITE_PLUGIN_DIFF_FILTER_TRIGGERED_DOC} == 1 ]]; then
But on that note: Do we always need at least one Julia build to also be able to build the docs? That's how I understand the instructions here #243 (comment). If so, then that's not yet handled here.
Perhaps I should revert the diff-filter-doc
for the moment, then just not ignore HISTROY.md
and NEWS.md
for diff-filter-build
, and always build the docs when we build julia. A follow-up PR could then experiment with refinements.
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, we need at least one build of Julia in order to do the docs job.
I would say that it's okay to just do that one build (as well as the docs build) unconditionally, regardless of the diff.
4cbd42b
to
98f6c0d
Compare
98f6c0d
to
724d2ee
Compare
This adds a filter that skips CI if a PR only changed things like
README.md, HISTROY.md
etc.This is a first attempt towards fixing #243.
Still lacks a few desired features like (list taken from the recent Slack thread, @DilumAluthge)
master
.