Conversation
|
Tests have not been added yet, and this has turned into quite a big PR, so I may have to split it. |
Sakib25800
force-pushed
the
port-unrolling
branch
from
d978af3 to
ee4e231
Compare
Needed for unroll queue tests where multiple try-perf workflows are created
Needed for unroll queue commit message parsing
Sakib25800
force-pushed
the
port-unrolling
branch
from
ee4e231 to
e23e9fd
Compare
|
This has turned into a monstrous PR somehow, so I definitely have to split it. |
There was a problem hiding this comment.
Thank you very much for working on this! I haven't read through it all yet, so I'll do the review piecemeal.
The second commit that refactors start_build looks like a nice self-contained cleanup. Could you please separate that into its own PR first? It touches a critical piece of bors code, so I want to land it separately and test it on production.
| &self, | ||
| rollup: &PullRequestModel, | ||
| members: &[PullRequestModel], | ||
| members: &[(PullRequestModel, CommitSha)], |
There was a problem hiding this comment.
Let's create a proper struct for it, something like RollupMember, it's not clear from the tuple what do the fields mean. There we can also document that the commit SHA is the HEAD SHA, not the rollup merge SHA.
|
Then the first commit would also be nice to land in a separate PR; I have a use-case unrelated to rollup unrolling where the SHA would be useful. |
2 participants
Fixes #285