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

Bump action fetches less files #62

Merged
merged 4 commits into from
Aug 7, 2024
Merged

Bump action fetches less files #62

merged 4 commits into from
Aug 7, 2024

Conversation

onefifth
Copy link
Contributor

@onefifth onefifth commented Aug 3, 2024

Summary

Tweaks the actions/checkout behavior in cloud-actions/og/bump to fetch the minimum number of commits required to do its job. Previously this was fetching all history for branches in the whole repo.
This action now only fetches:

  • commits that are in the PR (a.k.a. the commits between base and target)
    • used to check for manual, non-bot commits to the version file
  • the most-recent commit from the PRs base branch head
    • to scrape a current version number

Details

This change can save significant amounts of time during the checkout actions for large repos with many branches.
Additionally, it should no-longer be necessary to specify fetch-depth: 0 in checkout actions that precede the shared cloud-actions (unless some other custom workflow step needs this full git history). The changes here were heavily based on this comment.

References

See https://github.com/XanaduAI/gittagtest/pull/28 for tests

See real-world example on https://github.com/XanaduAI/chip-data-analysis/pull/196:
cloud-actions/bump@main : job link (1m38s)
cloud-actions/bump@bump_pulls_less_files: job link (3s)

Notes

I would encourage testing this by temporarily pointing other real workflows (ideally before opening a PR) to this cloud-actions branch to confirm there aren't any unforeseen issues.

Additionally, for workflows where the cloud-actions are preceded by an actions/checkout action with a fetch-depth: 0, you will not see any time savings until the initial fetch-depth argument is removed. Multiple repos I looked at currently do this. I'd recommend removing this argument while doing the aforementioned test.

Last minor note, there is a node deprecation warning caused by actions/checkout@v3. I've tested this change using v4 and had no issues, but bumping that dependency felt outside the scope of this already kinda complicated PR.

Copy link

@lneuhaus lneuhaus left a comment

Choose a reason for hiding this comment

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

Thanks!

@heltluke
Copy link
Contributor

heltluke commented Aug 5, 2024

I think our "plugin" repos tend to simply use

      - name: Cloud-action bump
        uses: xanaduAI/cloud-actions/og/bump@main

so this will speed them up without requiring any changes on our end. Does that sound right to you @lneuhaus?

Or should we be replacing the initial checkout at the top of each apply_sw4hw_standards.yml with this pattern to fully realise its benefits?

@onefifth
Copy link
Contributor Author

onefifth commented Aug 5, 2024

@heltluke Many CI workflows I looked at explicitly use the GitHub actions/checkout before this one. It should be safe to remove those and rely on the checkout embedded in this action instead.
Or, at the very least, removing any fetch-depth: 0 arguments from other workflows where possible. I think that arg started getting copy-pasted around as an easy way to make this complicated history-inspecting bump action work correctly, but it's a big waste of time in the vast majority of cases.

@onefifth
Copy link
Contributor Author

onefifth commented Aug 7, 2024

After some additional testing here https://github.com/XanaduAI/octopusgarden/pull/1621 (thanks @brandonaltieri1), I'm going ahead and merging this.

@onefifth onefifth merged commit f20d998 into main Aug 7, 2024
@onefifth onefifth deleted the bump_pulls_less_files branch August 7, 2024 17:51
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.

3 participants