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

ci to update all literature monthly #134

Merged
merged 4 commits into from
Dec 31, 2024
Merged

ci to update all literature monthly #134

merged 4 commits into from
Dec 31, 2024

Conversation

hdashnow
Copy link
Member

@hdashnow hdashnow commented Dec 30, 2024

This updates all citations monthly using GitHub Actions. I'm not sure if I have the logic of the triggers correct here.

Will resolve #59

Copy link

netlify bot commented Dec 30, 2024

Deploy Preview for strchive ready!

Name Link
🔨 Latest commit a50316a
🔍 Latest deploy log https://app.netlify.com/sites/strchive/deploys/6774846f3043530008a76848
😎 Deploy Preview https://deploy-preview-134--strchive.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

snakemake --config stages="new-refs"

- name: Update all literature (long)
if: ${{ github.event_name == 'schedule' || github.event_name == 'workflow_dispatch'}}
Copy link
Contributor

Choose a reason for hiding this comment

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

So the long and intermediate steps will both run on workflow dispatches? Just checking that this is what you intended.

Copy link
Contributor

@vincerubinetti vincerubinetti Dec 31, 2024

Choose a reason for hiding this comment

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

The reason I made this comment originally is because, the way they're named "short intermediate long", I would expect them to be mutually exclusive, i.e. only one of them runs each workflow run. But I haven't looked inside the scripts to see what's actually going on yet, is that actually the case? It doesn't have to be; I'd just recommend removing the lengths from the names if not.

Looks like now there's a clear split/association between the three trigger types (pr, schedule, manual) and the three script lengths (short, intermediate, long). Note that schedule and manual will also trigger PR on subsequent commits. Here's a breakdown of the scenarios (in terms of automatic workflow runs):

  • Someone commits to main → nothing happens
  • Someone manually creates a PR → someone commits → pull_request triggers, "short" script runs → updated data committed to PR
  • Someone manually runs workflow_dispatch → "intermediate" script runs → PR opened with first commit of updated data → someone commits again to opened PR → pull_request triggers → "short" script runs → updated data committed to PR
  • schedule runs once a month → "long" script runs → PR opened with first commit of updated data → someone commits again to opened PR → pull_request triggers → "short" script runs → updated data committed to PR

If that looks okay, then the PR LGTM

Copy link
Member Author

Choose a reason for hiding this comment

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

Thanks for breaking that down!
Yes, I think that is the desired behavior. Let's see if that all works out in practice.

.github/workflows/update-loci.yaml Outdated Show resolved Hide resolved
@hdashnow
Copy link
Member Author

Thanks for the feedback @vincerubinetti. Does this look better?

- name: Open pull request with updated files
if: ${{ !(github.event_name == 'pull_request') }}
if: ${{ !(github.event_name == 'pull_request') || github.event_name == 'schedule' }}
Copy link
Contributor

Choose a reason for hiding this comment

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

This "or schedule" is also unnecessary as it will already be covered by "not pull request"

@hdashnow hdashnow merged commit 078a4d7 into main Dec 31, 2024
2 checks passed
@hdashnow hdashnow deleted the ci-monthly branch December 31, 2024 23:57
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.

Set up CI
2 participants