Squash & Approve - #828
Conversation
7fbdf0b to
ab07984
Compare
Added a new command that can squash and, if successful, approve a the latest resulting commit Signed-off-by: David Manca <1900179+medzernik@users.noreply.github.com>
Signed-off-by: David Manca <1900179+medzernik@users.noreply.github.com>
ab07984 to
d1b84ce
Compare
|
Thanks! I'll try to fix the remaining errors in tests and get it cleaned up today or tommorow. |
Signed-off-by: medzernik <medzernik@medzernik.dev>
Signed-off-by: medzernik <medzernik@medzernik.dev>
There was a problem hiding this comment.
Thanks! Left some comments.
There is one race condition that we'll need to handle in handle_push_to_pull_request. If we push the squashed commit, GitHub will send us a webhook about the push, which will unapprove the PR in handle_push_to_pull_request. Of course that is not ideal if we just approved it after the squash :)
I will push some unrelated refactoring towards that.
| BorsCommand::Retry => {} | ||
| BorsCommand::Cancel => {} | ||
| BorsCommand::Squash { .. } => {} | ||
| BorsCommand::SquashAndApprove { .. } => {} |
There was a problem hiding this comment.
The command should be documented in the help message below.
| let pr_db = db2 | ||
| .get_pull_request(repo2.repository(), pr_github.number) | ||
| .await? | ||
| .expect("TODO"); |
There was a problem hiding this comment.
If this fails, we should return an Err from the callback, saying that the PR was not found in the DB.
|
|
||
| #[sqlx::test(migrator = "crate::MIGRATOR")] | ||
| async fn squash_two_commits_and_approve(pool: sqlx::PgPool) { | ||
| let pool = Arc::new(pool); |
| pr.reset_to_single_commit(Commit::from_sha("sha1")); | ||
| pr.add_commits(vec![Commit::from_sha("sha2")]); | ||
| }); | ||
| ctx.post_comment("@bors squash").await?; |
There was a problem hiding this comment.
This should be using @bors r+ squash, which is what we want to test, right?
| /// Squash message for the commit | ||
| commit_message: SquashCommitMessage, | ||
| }, | ||
| SquashAndApprove { |
There was a problem hiding this comment.
We need to actually parse the command 😆 I was surprised why it was doing nothing in tests.. 😆
2 participants
Added a Squash and Approve function. This should fix #691.
Please let me know anything I can do to improve the code if needed.
I would add that in
https://github.com/medzernik/bors/blob/ab07984372498f757ab06438e926a6a6c0424c2b/src/bors/handlers/squash.rs#L25-L27
I wanted to use some more differentiated error states, but with anyhow and the general practice of returning
Ok(X)even if an error technically occurs, I just wrapped theshain anOption<sha>Mentored by: @Kobzol