Skip to content

Quick workflow filter tweak #554

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

Merged
merged 6 commits into from
Nov 29, 2024
Merged

Quick workflow filter tweak #554

merged 6 commits into from
Nov 29, 2024

Conversation

validbeck
Copy link
Collaborator

@validbeck validbeck commented Nov 28, 2024

Internal Notes for Reviewers

Refer to #552:

I think I know why the comparison is failing — our branches essentially match between merges, so we actually have to compare the PR being merged to the base instead

documentation-workflows

Expected behaviour

PR > main

    • site/notebooks/** has a change pushed up to the PR, it will trigger the notebook execution: SUCCESSFUL WORKFLOW RUN

main > staging

  • After this PR is merged into main, the workflow merging into staging from main will trigger the notebook execution: WORKFLOW LINK PLACEHOLDER

staging > prod

  • The next time we publish to prod, the workflow merging into staging from prod will trigger the notebook execution: WORKFLOW LINK PLACEHOLDER

@validbeck validbeck added the internal Not to be externalized in the release notes label Nov 28, 2024
@validbeck validbeck self-assigned this Nov 28, 2024
@validbeck validbeck requested a review from nrichers November 28, 2024 00:55
Copy link
Contributor

PR Summary

This pull request updates the GitHub Actions workflows for deploying documentation to both production and staging environments. The changes involve modifying the head reference in the filter step of the workflows to use a dynamic reference ${{ github.head_ref }} instead of hardcoded branch names (staging for production and main for staging). This enhancement allows the workflows to dynamically determine the head reference, improving flexibility and reducing the need for manual updates when branch names change.

Test Suggestions

  • Verify that the GitHub Actions workflows trigger correctly with the new dynamic head reference.
  • Test the deployment process to both production and staging environments to ensure that the documentation is deployed as expected.
  • Check that the filter step correctly identifies changes in the 'site/notebooks/**' path with the new head reference.

Copy link
Contributor

A PR preview is available: Preview URL

Copy link
Contributor

A PR preview is available: Preview URL

Copy link
Collaborator

@nrichers nrichers left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM! 🚀

@validbeck
Copy link
Collaborator Author

Quickly edited our announcement banner to link to our new brief:

Screenshot 2024-11-28 at 1 48 38 PM

(Such a PITA that we can't single-source the announcements... believe me, I tried when I first implemented it...)

@nrichers
Copy link
Collaborator

Quickly edited our announcement banner to link to our new brief:

(Such a PITA that we can't single-source the announcements... believe me, I tried when I first implemented it...)

Looks great, single-sourced or not!

Copy link
Contributor

A PR preview is available: Preview URL

Copy link
Contributor

A PR preview is available: Preview URL

1 similar comment
Copy link
Contributor

A PR preview is available: Preview URL

@validbeck validbeck merged commit 90d91be into main Nov 29, 2024
5 checks passed
@validbeck validbeck deleted the beck/quick-workflow-fix branch November 29, 2024 00:48
@validbeck
Copy link
Collaborator Author

Omgggggggg it still doesn't work whyyyyyyyyyyyyyyy 🫠 I'm going to go insane, why did it work on staging the FIRST TIMEEEEEEE...

PR > Main: Consistently works.
Main > Staging: Worked the VERY 1st time when we had no additional base/ref direction so I am going to revert back to that.
Staging > Prod: This SHOULD work now but ughhh I'm gonna think about it a bit more before I try again. 😭

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
internal Not to be externalized in the release notes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants