feat: review command

[raffis]

What does this change

Hey me again 🙈 ,

This pr adds the ability for approving pull requests supported by all platforms.
Basically reviewers now get an easy option to approve pr's made by multi-gitter (or other(s)).
Usually pull requests require approval in branch protected repos and there is currently no good way to batch approve multi pr's.
For example we collect all batch operations in a repository, a reviewer can grab the config from there and start a straight forward review process with the same tool. (Using the --batch option this could even be fully automated in a ci pipeline).

This pr adds the review command with a couple of options:

• --comment for a review comment (can also be overwritten while reviewing pr's)
• --all to batch review all of them. Basically review a diff output from all pr's. Useful if you have 100 pr's with a single line change where you don't want to review each individually.
• --batch=x Approve/Decline/Comment all targeted pull requests without asking questions
• --no-pager This pr pipes the diffs to a pager (less, more (windows), PAGER env). If no pager should be used one can dump to stdout directly using this option
• --include-approved By default approve ignores pull requests which are already approved by the reviewing person.

example using vim with syntax highlighting:

 PAGER=vim multi-gitter review --log-level=debug --config config.yaml -c "lgtm" --all

What issue does it fix

Currently reviewing is hard. If you open 100 pr's with a simple change you either have to wait until people reviewed them, try to push them or whatever... There are also some workarounds like verify 2 and then batch approve everything via the api.

Notes for the reviewer

• No tests added so far, waiting for your comments if this will be merged.
• I did not test it on windows yet
• I was unable to test bitbucket so far

Checklist

• Made sure the PR follows the CONTRIBUTING.md guidelines
• Tests if something new is added

[lindell]

Hey. Thanks for the PR! But please create an issue discussion about the addition so that you don't create anything unnecessarily ;)

I do think this is a good addition but will have to take some time before looking at it since the changes are extensive. Let me get back to you with a review in a few days :)

[raffis]

Hey. Thanks for the PR! But please create an issue discussion about the addition so that you don't create anything unnecessarily ;)

I do think this is a good addition but will have to take some time before looking at it since the changes are extensive. Let me get back to you with a review in a few days :)

Yeah should have commented (fixes #295). Sure take your time 👍🏻

[lindell]

Some initial comments. I've not reviewed all files yet.

[lindell]

Is it possible to use batch without all, or the reverse?

[raffis]

Batch always refers to all pulls, so it includes --all not matter if a user would set it or not (It's describe like that in the flag helper). While you can do all without the batch option. If all is set without batch a user gets the diff for all pull requests at once.

[lindell]

I think batch should be named something else. state maybe? As it's called that at GitHub

[lindell]

Whatever it is called, it should be converted to an "enum" here, and therefore verified to be correct, before calling out to the multigitter package.

See conflict strategy as an example

[raffis]

Agreed with the enum, definitely makes sense 👍🏻

With the flag name, from a user perspective and cli tooling I would pick batch as its more obvious that something is going on which does not require user interaction which is not the case if it's called state. It could be --batch and --state=x. But thats an additional flag for no reason.

[lindell]

The problem is that other settings (comments) will also be applied as a batch.
The state is called batch because it can be applied to all PRs, but comments will still be called comment, even if it will still be applied in batch.

If state where to be used, it should be required to use with --all. --all could still be used on it's own, and it would then ask for then ask for the state (and comment).

[raffis]

I think --batch is more common to understand as a user that no additional follow up questions are required and it can be used silently. Might also call it silent instead.
If batch is just ditched and its called state I think it's not exactly what it's intended for.

[raffis]

Just used this new command to batch review and merge hundred pr's opened by renovate (not created using multi-gitter itself). Thats also a nice use case for this command.

[lindell]

The problem is that other settings (comments) will also be applied as a batch.
The state is called batch because it can be applied to all PRs, but comments will still be called comment, even if it will still be applied in batch.

If state where to be used, it should be required to use with --all. --all could still be used on it's own, and it would then ask for then ask for the state (and comment).

[v1gnesh]

Hi @lindell,

Firstly thank you for sharing your excellent work that is this tool.
Is there anything else left to do with this PR before you would merge it?
This will be super handy for the core maintainers of the tools under https://github.com/zosOpenTools/