-
Notifications
You must be signed in to change notification settings - Fork 54
Add condition to not notify slack when don't have files in dags folder #792
Add condition to not notify slack when don't have files in dags folder #792
Conversation
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.
Thank you for submitting this PR @ilitotor! This is a great approach. It looks like the linting is failing, I think the proposed suggestion below should address the issue 😄
Co-authored-by: Madison Swain-Bowden <[email protected]>
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.
The new changes look great, but apologies, one more thing before this is ready!
dag-sync.sh
Outdated
|
||
# Verify if have /dags/ in the last commit | ||
have_dag=$(git log -p -1 "$new" --pretty=format: --name-only | grep "/dags/") | ||
# If there is no files under /dags/ folder, no need to notify, quit early | ||
[ -z "$have_dag" ] && exit |
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.
One more change I just realized (sorry!), this should go after the git reset --hard "$new"
line below. If we don't move forward on each commit, $new
will always be the next commit ahead and we will run into the same loop of logic for skipping it if it happens to not include DAGs. We only want to exit prior to sending the Slack notification if there aren't any DAGs.
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.
Looks good. Thank you for this valuable contribution to the Openverse's catalog @ilitotor!
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.
Looks great, congrats on your first contribution! 😄
Fixes
Fixes WordPress/openverse#1408 785 by @AetherUnbound
Description
This PR gets the last git log and checks if have files inside the
dags
folder to notify slack.Checklist
Update index.md
).main
) or a parent feature branch.Developer Certificate of Origin
Developer Certificate of Origin