-
Notifications
You must be signed in to change notification settings - Fork 1.7k
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
Fix #9385: Add a workflow to check if artifacts have changed #9553
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #9553 +/- ##
==========================================
+ Coverage 87.96% 87.98% +0.01%
==========================================
Files 167 167
Lines 22168 22168
==========================================
+ Hits 19501 19505 +4
+ Misses 2667 2663 -4
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
Thank you for your pull request! We could not find a changelog entry for this change. For details on how to document a change, see the contributing guide. |
if: env.CHANGED == 'true' && !contains(github.event.pull_request.labels.*.name, 'artifact_minor_upgrade') | ||
run: | | ||
echo "CI failure: Artifact changes checked in core/dbt/artifacts directory." | ||
echo "To bypass this check, add the 'artifact_minor_upgrade' label to the PR." |
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.
Could you update this message to include a link to the breaking changes section of the dbt/artifacts/README.md?
Perhaps something like: "To bypass this check, confirm that the change is not breaking (link) and add the 'artifact_minor_upgrade' label to the PR."
id: check_changes | ||
run: | | ||
# Check for changes in dbt/artifacts | ||
CHANGED_FILES=$(git diff --name-only origin/${GITHUB_BASE_REF} origin/${GITHUB_HEAD_REF} | grep 'core/dbt/artifacts/') |
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.
Could we exclude matching core/dbt/artifacts/README.md
as part of this search?
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.
We're ignoring all *.md
files, so we should be fine:
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.
couple small comments re: linking to and ignoring changes to core/dbt/artifacts/README.md, but otherwise looks great! thank you!
resolves #9385
Problem
It isn't always clear when a file in the
core/dbt/artifacts
directory has been changed.Solution
This PR adds a CI check that:
core/dbt/artifacts
has been changedartifact_minor_upgrade
tag to the PR.This solution currently uses
git diff
and doesn't rely on any external actions.Run history:
Checklist