Skip to content

CI: Prevent duplicate CI runs#1733

Merged
apoelstra merged 1 commit intorust-bitcoin:masterfrom
junderw:ci-prevent-double
Mar 22, 2023
Merged

CI: Prevent duplicate CI runs#1733
apoelstra merged 1 commit intorust-bitcoin:masterfrom
junderw:ci-prevent-double

Conversation

@junderw
Copy link
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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
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