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

Update workflow for PRs #45

Merged
merged 3 commits into from
Nov 4, 2024
Merged

Update workflow for PRs #45

merged 3 commits into from
Nov 4, 2024

Conversation

webbnh
Copy link
Contributor

@webbnh webbnh commented Oct 31, 2024

The current workflow is broken when triggered by a PR event (see arcalot/arcaflow-plugin-rtla#1): the GHA checkout uses a detached head from a temporary branch, and so the attempt to push the update fails. (When triggered by a branch update event, it runs on a "real branch", and there's no problem.)

This PR tweaks the update step of the workflow:

  • after configuring Git but before making any changes, it switches to the the "head branch" from the PR instead of using the current checkout
  • then it moves the updated contents into the README.md file and commits it to the branch
  • then, after pushing the branch, it switches the checkout back to the original ref

These steps are taken only if the trigger was a pull request (and only if there is a difference in the generated documentation). The value of github.head_ref is only available for PRs, so none of this will work for other triggers. And, more importantly, I suspect that we don't want this step to run except for PRs -- that is, I don't think we want to bot making changes to branches when no one is supervising. (The current state disables this step if the branch is protected, but that doesn't seem strict enough to me.)

This PR also streamlines the difference check in a separate commit.

Note: until this PR is merged, you can try it out by modifying your plugin workflow to reference @PR-support instead of @main.


By contributing to this repository, I agree to the contribution guidelines.

Copy link

@dbutenhof dbutenhof left a comment

Choose a reason for hiding this comment

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

Hmm. That's ... interesting.

.github/workflows/reusable_workflow.yaml Outdated Show resolved Hide resolved
@webbnh webbnh marked this pull request as ready for review October 31, 2024 20:31
@webbnh
Copy link
Contributor Author

webbnh commented Oct 31, 2024

I tested this using arcalot/arcaflow-plugin-rtla#2 and, once I got the wrinkles smoothed out, the docsgen run was able to update my branch and the workflow was able to reset the checkout to the original (which does not contain the update...but it was safe on my branch).

So, @dustinblack, you should be able to pop 5007ae2 and eb58c53 from your branch, and when you push the result up the bot should push the updates back on. (Assuming that your PR uses this PR's updated workflow.)

@webbnh
Copy link
Contributor Author

webbnh commented Oct 31, 2024

I'll give @dustinblack a chance to look and comment before this is merged. (Dustin, feel free to do the merge if you are good with this change.)

@dustinblack dustinblack merged commit c728508 into main Nov 4, 2024
3 checks passed
@dustinblack dustinblack deleted the PR-support branch November 4, 2024 13:16
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.

4 participants