search

ejoffe / spr

Stacked Pull Requests on GitHub
MIT License
796 stars 68 forks source link

Feature request: Handle non-assignable reviewers more gracefully #220

Open yogurtearl opened 2 years ago

yogurtearl commented 2 years ago

if you run this command, but jane is not an assignable reviewer, then it just errors out and doesn't create the rest of the PRs. 

git spr u -r bob -r jane -r alice

Current behaviour:

Stops after the first PR and doesn't assign reviewers to that first PR. Also, doesn't print the status of the PR it did create.

Desired behaviour:

Creates all the PRs with only bob and alice as reviewers, and a warning message about jane.

wwade commented 2 years ago

I think it's better to error out instead of printing warnings. It's easy enough to simply fix the typo (or drop the review, if that's somehow the right choice) and rerun the command. This is, of course, predicated on the fix for the bug you pointed out here.

Warnings are pretty easy to miss, especiall in the git spr output as it's fairly verbose already.

My vote is that the desired behaviour is to error out early instead of creating any PRs so that the error can be fixed.

yogurtearl commented 2 years ago

My vote is that the desired behaviour is to error out early instead of creating any PRs so that the error can be fixed.

that would be fine as well. and mitigated by https://github.com/ejoffe/spr/issues/221 as well.