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: Prevent duplicate CI runs #1733

Merged
merged 1 commit into from Mar 22, 2023

Conversation

junderw
Copy link
Contributor

@junderw junderw commented Mar 22, 2023

Closes #1726

This will prevent duplicate CI runs when merging an internal branch into master.

You can opt-in to a non-master branch CI by using the prefix given.

@apoelstra
Copy link
Member

This will prevent duplicate CI runs when merging an internal branch into master.

For what it's worth, we never do this in this repo (without going through a PR, anyway). But we do sometimes have long-lived branches or experimental forks, and it would be nice to avoid CI churn on those.

Copy link
Member

@apoelstra apoelstra left a comment

Choose a reason for hiding this comment

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

ACK a629bef

Copy link
Collaborator

@Kixunil Kixunil left a comment

Choose a reason for hiding this comment

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

ACK a629bef

on:
push:
branches:
- master
Copy link
Collaborator

Choose a reason for hiding this comment

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

We might also want 0.29.x (and other version branches), what do you think @apoelstra?

Copy link
Member

Choose a reason for hiding this comment

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

On those branches, can we run regular CI except the 1.41 tests, because those typically break soon after release, and it's annoying to backport cargo update commands. Similarly we'd want to disable clippy and fmt, which drift over time.

I think this'll be a bit more complicated, so I'll move to merge this PR and we can do that stuff in a later one.

Copy link
Member

Choose a reason for hiding this comment

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

I also think that such a change wouldn't actually take effect unless we backport it to the release branches.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, that is correct. We would need to backport to any branches that want these changes applied.

Copy link
Collaborator

Choose a reason for hiding this comment

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

I think we could just run with locked dependencies, so not cargo update commands need to be backported. fmt and clippy could be tested with pinned versions.

@apoelstra
Copy link
Member

Ah, I think I understand now where the duplication is -- when we do pull requests, this triggers a CI run both on the tip of the PR branch (the "push" job) and on the merge commit (the "pull request" job). We don't want to do this.

If we wanted to CI-check individual commits, we could do that, but we should do it on purpose (and not do the full CI matrix, just whatever we want for bisection) (and actually do it on all the commits, not just the tip).

@apoelstra apoelstra merged commit dda3cb6 into rust-bitcoin:master Mar 22, 2023
22 checks passed
This was referenced Mar 22, 2023
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.

Suggestion for CI: Only run on push to master OR pull request
3 participants