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.

Sign up for GitHub

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

Jump to bottom

Speed up CI with a two-tier system built on bors #2801

Closed
epage opened this issue Oct 1, 2021 Discussed in #2723 · 2 comments · Fixed by #2802
Closed

Speed up CI with a two-tier system built on bors #2801

epage opened this issue Oct 1, 2021 Discussed in #2723 · 2 comments · Fixed by #2802

Comments

@epage
Copy link
Member

epage commented Oct 1, 2021

Discussed in #2723

Originally posted by epage August 18, 2021
clap tests a lot of system combinations

Pros

  • Ensure we don't break clap for any of our users

Cons

  • This makes it easy to max out our number of parallel runners, slowing down CI results for PRs
  • While Github offers hosting for free, we should be mindful of the cost and be respectful with the resources we use

However, with bors, we can get the best of both worlds. As a merge queue, bors does one extra CI check before commits make it to master. We can rely on that to implement a two-tier CI system

  • Checks that run on pull requests
  • Checks that run on branches, like master, staging, and trying

If a contributor suspects their change would impact a case not covered for pull requests, they can comment bors try on their PR (I'm assuming try is available to contributors, its unclear who has access rights to each command).

So this means for each push to a PR, we can run a fraction of the jobs, completing much more quickly while still verifying nothing will make master red by testing during the merge proces. If it would, it gets kicked out and the PR author gets that feedback.

Proposed implementation:

  • Split ci.yml into pr.yml and branch.yml
  • pr.yml would not include
    • wasm
    • i686 procs (clap generally doesn't have proc-specific logic)
    • macos-latest (close enough to ubuntu-latest for what clap does)
    • rust beta and nightly (changes are unlikely to break with upcoming Rust versions but instead with MSRV)

Additional speed ups
epage added a commit to epage/clap that referenced this issue Oct 1, 2021
@epage
WIP: Speed up PR feedback
24a2d24 
This drops us down to just a handlful of jobs, allowing us to full
parallelism (github caps max parallel jobs).

There is a balance in what to run.  We should consider what is most
likely to break for the widest variety of PRs.  Contributors that expect
an uncovered case to fail can always specify `@bors try`

Motivation
- Mac is similar enough to Linux, we only need to run one of them and
  Linux has more parallel runners on Github.
- Since we deal with `OsStr`, test Windows because its different than
  the others.
- People are most likely to make changes on `stable` and break support
  for MSRV, so we should verify that
- Still test on `stable` to not block feedback if we run into problems
  with dependencies and our MSRV run.
- On the other hand, beta and nightly are less likely to break on an
  individual PR.

This is depednent on us using bors to run the ci-full before merging
into master.

Fixes clap-rs#2801
@epage epage mentioned this issue Oct 1, 2021
Merged
epage added a commit to epage/clap that referenced this issue Oct 1, 2021
@epage
WIP: Speed up PR feedback
e2b510d 
This drops us down to just a handlful of jobs, allowing us to full
parallelism (github caps max parallel jobs).

There is a balance in what to run.  We should consider what is most
likely to break for the widest variety of PRs.  Contributors that expect
an uncovered case to fail can always specify `@bors try`

Motivation
- Mac is similar enough to Linux, we only need to run one of them and
  Linux has more parallel runners on Github.
- Since we deal with `OsStr`, test Windows because its different than
  the others.
- People are most likely to make changes on `stable` and break support
  for MSRV, so we should verify that
- Still test on `stable` to not block feedback if we run into problems
  with dependencies and our MSRV run.
- On the other hand, beta and nightly are less likely to break on an
  individual PR.

This is depednent on us using bors to run the ci-full before merging
into master.

Fixes clap-rs#2801
@epage
Copy link
Member Author

epage commented Oct 1, 2021

@pksunkara you mentioned in #2723 that you were going to look into this. I was already planning on looking into it (#2802) but the PoC cost is low enough that I think its worth us both playing with our ideas and communicating through our respective PRs. We can see how they turn out and pick the best from both.

@pksunkara
Copy link
Member

Your PR is pretty good, just a few things I wanted to be addressed.

epage added a commit to epage/clap that referenced this issue Oct 1, 2021
@epage
WIP: Speed up PR feedback
b1d69b4 
This drops us down to just a handlful of jobs, allowing us to full
parallelism (github caps max parallel jobs).

There is a balance in what to run.  We should consider what is most
likely to break for the widest variety of PRs.  Contributors that expect
an uncovered case to fail can always specify `@bors try`

Motivation
- Mac is similar enough to Linux, we only need to run one of them and
  Linux has more parallel runners on Github.
- Since we deal with `OsStr`, test Windows because its different than
  the others.
- People are most likely to make changes on `stable` and break support
  for MSRV, so we should verify that
- Still test on `stable` to not block feedback if we run into problems
  with dependencies and our MSRV run.
- On the other hand, beta and nightly are less likely to break on an
  individual PR.

This is depednent on us using bors to run the ci-full before merging
into master.

Fixes clap-rs#2801
epage added a commit to epage/clap that referenced this issue Oct 2, 2021
@epage
WIP: Speed up PR feedback
99995f6 
This drops us down to just a handlful of jobs, allowing us to full
parallelism (github caps max parallel jobs).

There is a balance in what to run.  We should consider what is most
likely to break for the widest variety of PRs.  Contributors that expect
an uncovered case to fail can always specify `@bors try`

Motivation
- Mac is similar enough to Linux, we only need to run one of them and
  Linux has more parallel runners on Github.
- Since we deal with `OsStr`, test Windows because its different than
  the others.
- People are most likely to make changes on `stable` and break support
  for MSRV, so we should verify that
- Still test on `stable` to not block feedback if we run into problems
  with dependencies and our MSRV run.
- On the other hand, beta and nightly are less likely to break on an
  individual PR.

This is depednent on us using bors to run the ci-full before merging
into master.

Fixes clap-rs#2801
epage added a commit to epage/clap that referenced this issue Oct 2, 2021
@epage
WIP: Speed up PR feedback
52b58b8 
This drops us down to just a handlful of jobs, allowing us to full
parallelism (github caps max parallel jobs).

There is a balance in what to run.  We should consider what is most
likely to break for the widest variety of PRs.  Contributors that expect
an uncovered case to fail can always specify `@bors try`

Motivation
- Mac is similar enough to Linux, we only need to run one of them and
  Linux has more parallel runners on Github.
- Since we deal with `OsStr`, test Windows because its different than
  the others.
- People are most likely to make changes on `stable` and break support
  for MSRV, so we should verify that
- Still test on `stable` to not block feedback if we run into problems
  with dependencies and our MSRV run.
- On the other hand, beta and nightly are less likely to break on an
  individual PR.

This is depednent on us using bors to run the ci-full before merging
into master.

Fixes clap-rs#2801
epage added a commit to epage/clap that referenced this issue Oct 2, 2021
@epage
WIP: Speed up PR feedback
a06f69e 
This drops us down to just a handlful of jobs, allowing us to full
parallelism (github caps max parallel jobs).

There is a balance in what to run.  We should consider what is most
likely to break for the widest variety of PRs.  Contributors that expect
an uncovered case to fail can always specify `@bors try`

Motivation
- Mac is similar enough to Linux, we only need to run one of them and
  Linux has more parallel runners on Github.
- Since we deal with `OsStr`, test Windows because its different than
  the others.
- People are most likely to make changes on `stable` and break support
  for MSRV, so we should verify that
- Still test on `stable` to not block feedback if we run into problems
  with dependencies and our MSRV run.
- On the other hand, beta and nightly are less likely to break on an
  individual PR.

This is depednent on us using bors to run the ci-full before merging
into master.

Fixes clap-rs#2801
epage added a commit to epage/clap that referenced this issue Oct 2, 2021
@epage
WIP: Speed up PR feedback
6075705 
This drops us down to just a handlful of jobs, allowing us to full
parallelism (github caps max parallel jobs).

There is a balance in what to run.  We should consider what is most
likely to break for the widest variety of PRs.  Contributors that expect
an uncovered case to fail can always specify `@bors try`

Motivation
- Mac is similar enough to Linux, we only need to run one of them and
  Linux has more parallel runners on Github.
- Since we deal with `OsStr`, test Windows because its different than
  the others.
- People are most likely to make changes on `stable` and break support
  for MSRV, so we should verify that
- Still test on `stable` to not block feedback if we run into problems
  with dependencies and our MSRV run.
- On the other hand, beta and nightly are less likely to break on an
  individual PR.

This is depednent on us using bors to run the "ci" before merging
into master.

Fixes clap-rs#2801
epage added a commit to epage/clap that referenced this issue Oct 2, 2021
@epage
WIP: Speed up PR feedback
670eb54 
This drops us down to just a handlful of jobs, allowing us to full
parallelism (github caps max parallel jobs).

There is a balance in what to run.  We should consider what is most
likely to break for the widest variety of PRs.  Contributors that expect
an uncovered case to fail can always specify `@bors try`

Motivation
- Mac is similar enough to Linux, we only need to run one of them and
  Linux has more parallel runners on Github.
- Since we deal with `OsStr`, test Windows because its different than
  the others.
- People are most likely to make changes on `stable` and break support
  for MSRV, so we should verify that
- Still test on `stable` to not block feedback if we run into problems
  with dependencies and our MSRV run.
- On the other hand, beta and nightly are less likely to break on an
  individual PR.

This is depednent on us using bors to run the "ci" before merging
into master.

Fixes clap-rs#2801
epage added a commit to epage/clap that referenced this issue Oct 2, 2021
@epage
WIP: Speed up PR feedback
6e35bd2 
This drops us down to just a handlful of jobs, allowing us to full
parallelism (github caps max parallel jobs).

There is a balance in what to run.  We should consider what is most
likely to break for the widest variety of PRs.  Contributors that expect
an uncovered case to fail can always specify `@bors try`

Motivation
- Mac is similar enough to Linux, we only need to run one of them and
  Linux has more parallel runners on Github.
- Since we deal with `OsStr`, test Windows because its different than
  the others.
- People are most likely to make changes on `stable` and break support
  for MSRV, so we should verify that
- Still test on `stable` to not block feedback if we run into problems
  with dependencies and our MSRV run.
- On the other hand, beta and nightly are less likely to break on an
  individual PR.

This is depednent on us using bors to run the "ci" before merging
into master.

Fixes clap-rs#2801
epage added a commit to epage/clap that referenced this issue Oct 4, 2021
@epage
Speed up PR feedback
6a8c158 
This drops us down to just a handlful of jobs, allowing us full
parallelism (github caps max parallel jobs).  This is dependent on us
using bors to run the "ci" before merging into master.

There is a balance in what to run.  We should consider what is most
likely to break for the widest variety of PRs.  Contributors that expect
an uncovered case to fail can always specify `@bors try`

Motivation
- Mac is similar enough to Linux, we only need to run one of them and
  Linux has more parallel runners on Github.
- Since we deal with `OsStr`, test Windows because its different than
  the others.
- People are most likely to make changes on `stable` and break support
  for MSRV, so we should verify that
- Still test on `stable` to not block feedback if we run into problems
  with dependencies and our MSRV run.
- On the other hand, beta and nightly are less likely to break on an
  individual PR
- Remove benchmarks because most changes are not performance sensitive
  and we aren't looking at the results enough to justify a 30 minute run.

Fixes clap-rs#2801
epage added a commit to epage/clap that referenced this issue Oct 4, 2021
@epage
Speed up PR feedback
45d7ef8 
This drops us down to just a handlful of jobs, allowing us full
parallelism (github caps max parallel jobs).  This is dependent on us
using bors to run the "ci" before merging into master.

There is a balance in what to run.  We should consider what is most
likely to break for the widest variety of PRs.  Contributors that expect
an uncovered case to fail can always specify `@bors try`

Motivation
- Mac is similar enough to Linux, we only need to run one of them and
  Linux has more parallel runners on Github.
- Since we deal with `OsStr`, test Windows because its different than
  the others.
- People are most likely to make changes on `stable` and break support
  for MSRV, so we should verify that
- Still test on `stable` to not block feedback if we run into problems
  with dependencies and our MSRV run.
- On the other hand, beta and nightly are less likely to break on an
  individual PR
- Remove benchmarks because most changes are not performance sensitive
  and we aren't looking at the results enough to justify a 30 minute run.

Fixes clap-rs#2801
epage added a commit to epage/clap that referenced this issue Oct 6, 2021
@epage
Speed up PR feedback
a5b81ff 
This drops us down to just a handlful of jobs, allowing us full
parallelism (github caps max parallel jobs).  This is dependent on us
using bors to run the "ci" before merging into master.

There is a balance in what to run.  We should consider what is most
likely to break for the widest variety of PRs.  Contributors that expect
an uncovered case to fail can always specify `@bors try`

Motivation
- Mac is similar enough to Linux, we only need to run one of them and
  Linux has more parallel runners on Github.
- Since we deal with `OsStr`, test Windows because its different than
  the others.
- People are most likely to make changes on `stable` and break support
  for MSRV, so we should verify that
- Still test on `stable` to not block feedback if we run into problems
  with dependencies and our MSRV run.
- On the other hand, beta and nightly are less likely to break on an
  individual PR
- Remove benchmarks because most changes are not performance sensitive
  and we aren't looking at the results enough to justify a 30 minute run.

Fixes clap-rs#2801
epage added a commit to epage/clap that referenced this issue Oct 7, 2021
@epage
Speed up PR feedback
63275d3 
This drops us down to just a handlful of jobs, allowing us full
parallelism (github caps max parallel jobs).  This is dependent on us
using bors to run the "ci" before merging into master.

There is a balance in what to run.  We should consider what is most
likely to break for the widest variety of PRs.  Contributors that expect
an uncovered case to fail can always specify `@bors try`

Motivation
- Mac is similar enough to Linux, we only need to run one of them and
  Linux has more parallel runners on Github.
- Since we deal with `OsStr`, test Windows because its different than
  the others.
- People are most likely to make changes on `stable` and break support
  for MSRV, so we should verify that
- Still test on `stable` to not block feedback if we run into problems
  with dependencies and our MSRV run.
- On the other hand, beta and nightly are less likely to break on an
  individual PR
- Remove benchmarks because most changes are not performance sensitive
  and we aren't looking at the results enough to justify a 30 minute run.

Fixes clap-rs#2801
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Speed up PR feedback epage/clap
2 participants
@epage @pksunkara