Skip to content
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

Post jobs to slack when PR is merged #2

Merged
merged 3 commits into from
Nov 3, 2021
Merged

Conversation

jhkennedy
Copy link
Contributor

This updates the post-jobs-slack.yaml workflow to be based on pushes to main, so that jobs are posted to slack when a jobs PR is merged to ensure what's posted to the website, is what's posted to slack.

The command to get the parent commit hash:

PARENT_HASH=$(git log --pretty=%P -n 1 ${CURRENT_SHA} | cut -d ' ' -f 1)

works for both merge commits (e.g., USRSE/usrse.github.io@f4853c5) and squash commits/direct commits (e.g., USRSE/usrse.github.io@284fbb3).

I also checked to make sure it won't post when expired jobs get pruned.

@vsoch
Copy link
Contributor

vsoch commented Nov 3, 2021

Awesome! So my thinking is that we can just test this here - the CI should still be hooked up to a dummy slack that I made. When you are ready to do that I’m good to merge! And don’t worry if there are bugs and you need to tweak, perfection is the enemy of the food sometimes.

@vsoch
Copy link
Contributor

vsoch commented Nov 3, 2021

@jhkennedy one thing I just thought of - if you want to have this trigger on merge to another branch (your testing branch!) we can test this and get it into working condition before merge into main, just to make the git history a little cleaner. What do you think?

@jhkennedy
Copy link
Contributor Author

@vsoch This should be ready for review now.

I went ahead and added a slack integration to one of the workspaces I admin to test it out.

To test, I fast-forward merged this to themain branch of jhkennedy/jobs-updater, then

Both were successfully posted to my slack workspace:
image

@jhkennedy jhkennedy marked this pull request as ready for review November 3, 2021 05:46
@vsoch
Copy link
Contributor

vsoch commented Nov 3, 2021

This looks great, and thank you for testing! Let’s merge here, and then how about we update your PR to USRSE to use it? I would make sure to add a note to squash and merge (so there is just one new commit on merge to main) to the README. I am not an admin on the USRSE slack so you will need to ask someone to setup the integration for us. Great work tonight, I’m really excited about this!

@vsoch vsoch merged commit 68f712f into rseng:main Nov 3, 2021
@jhkennedy
Copy link
Contributor Author

@vsoch will do! I don't think squash merge is required though -- this should grab the parent on the main branch, so it doesn't matter if it's a merge commit, or just a (squashed) commit.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants