I've just added also the following info on (git)addrev in review-tools/README:

The tool accesses databases on www.openssl.org via HTTPS.
The environment variable 'https_proxy' can be used to connect via a proxy.
The transfer may take many seconds, in particular with the '--list' option.

While you're tweaking, would it be possible to have $MERGE default to "no"? The mere fact that this script defaults to squashing all commits into one has had me avoid it like the plague.

A strong "yes" to defaulting $MERGE to no.

While you're tweaking, would it be possible to have $MERGE default to "no"? The mere fact that this script defaults to squashing all commits into one has had me avoid it like the plague.

I was considering that as well but was reluctant because of the name of the tool.
Yet since there is strong consensus on changing the default I've done it now.

A strong "yes" to defaulting $MERGE to no.

How about changing the name of the tool to ghpush?
This would reflect its semantics better, covering both the merge and rebase case.

A strong "yes" to defaulting $MERGE to no.

How about changing the name of the tool to ghpush?
This would reflect its semantics better, covering both the merge and rebase case.

I would leave the name as it is, because the main action is the merging (in the sense of integration) of the pull request into the master branch. That we integrate it linearly without creating an explicit merge commit is more a technical detail IMHO.

Essentially, it's a git merge --ff-only.

A strong "yes" to defaulting $MERGE to no.

How about changing the name of the tool to ghpush?
This would reflect its semantics better, covering both the merge and rebase case.

I would leave the name as it is, because the main action is the merging (in the sense of integration) of the pull request into the master branch. That we integrate it linearly without creating an explicit merge commit is more a technical detail IMHO.

Looking at it once more, I think it's $MERGE which is misnamed, not ghmerge. I would suggest to name it $SQUASH and let it default to false.

Looking at it once more, I think it's $MERGE which is misnamed, not ghmerge. I would suggest to name it $SQUASH and let it default to false.

I'm not so sure on this, since in both cases some (auto)squashing is done:
git merge --no-commit --squash $WORK
vs.
git rebase -i --autosquash

Doing my most recent push using the new version of the tool,
I found it makes more sense to do the git rebase -i --autosquash before addrev is called.
I've changed this and also added an initial git pull on master.

Looking at it once more, I think it's $MERGE which is misnamed, not ghmerge. I would suggest to name it $SQUASH and let it default to false.

I'm not so sure on this, since in both cases some (auto)squashing is done:
git merge --no-commit --squash $WORK
vs.
git rebase -i --autosquash

It's a matter of how you view it: If the pull request has seven commits, each of which has several fixups, then the latter does a dumb squash-all-in-a-big-blob, while the former leaves the seven commits intact and only incorparates the fixups. But it's up to you to choose a name which suits you.

If the pull request has seven commits, each of which has several fixups, then the latter does a dumb squash-all-in-a-big-blob, while the former leaves the seven commits intact and only incorparates the fixups.

... but the other way around. --squash is the "dumb squash-all-in-a-big-blob", while --autosquash "leaves the seven commits intact and only incorparates the fixups."

... but the other way around.

Yeah, thanks for noticing. I mixed up the order.

It's a matter of how you view it: If the pull request has seven commits, each of which has several fixups, then the former does a dumb squash-all-in-a-big-blob, while the latter leaves the seven commits intact and only incorporates the fixups.

[in this quote I've fixed the confusion mentioned above]

I know.

But it's up to you to choose a name which suits you.

All right, so I've kept the variable name MERGE
but renamed the option '-nosquash' to the meanwhile more to-the-point '-autosquash'.

Please reword the commit message of e738545 to add an empty line between 'various tweaks' and the subitems

Done.

Unfortunately, such a change breaks the autosquash capability of the fixups for this commit because git references the base commit by its title (and not, e.g. by its hash-based ID).

In the future I'll make sure to put an empty line after the first line of multi-line commit messages.

git references the base commit by its title (and not, e.g. by its hash-based ID).

GIt does not reference the commit hash by design. Just imagine that git would do that. Then it would have to rewrite the commit messages of all fixups every time you rebase.
Fortunately, it's not such a big deal to reorder and edit the rebase-todo list manually, so an interactive rebase is still feasible :-)

I've just added --ff-only to the git merge invocations to be on the safe side.

I suggest you avoid aliases, you don't need more than one way to do things.

I've meanwhile cleaned up up the (fixup) commits I've done so far.
All comments/requets have been addressed, also the formal change request by @mspncp.

I've done some further small improvements on ghmerge and its documentation.

I found that sometimes (after some more complex/messed-up commits that interfere with each other) the --autosquash option of interactive rebase re-arranges commits in a way that causes merge conflicts. Therefore I've fixed the cleanup behavior of ghmerge in this case and added the --noautosquash option that leaves out --autosquash on git rebase -i.

I've just added a further option to ghmerge:
using --remote <arg> one can select the git remote of the branch to pull from and push to.
This defaults to 'origin', the hitherto hard-wired value.

I've also slightly simplified the CLI of addrev:
Instead of --prnum=NNN' one can now simply give NNN`, e.g.:

addrev 1234 -2 steve @richsalz

This defaults to 'origin', the hitherto hard-wired value.

Why not ask git instead of assuming 'origin'?

$ git remote -v | grep 'github.com/openssl/openssl'
github https://github.com/openssl/openssl.git (fetch)
github https://github.com/openssl/openssl.git (push)

Ups, wrong repo :-)

$ git remote -v | grep git.openssl.org
origin git://git.openssl.org/openssl.git (fetch)
origin [email protected]:openssl.git (push)

A classical use-case for good'ol awk:

$ git remote -v | awk '/git.openssl.org.*(push)/{ print $1; }'
origin

$ git remote -v | awk '/git.openssl.org.*(push)/{ print $1; }'
origin

@mspncp, good thought and thanks for the code snippet.
I've just extended it by | head -n 1 for the case that more than just one such remote is listed.
I've updated also the README in this PR accordingly.
Again all your change requests are fulfilled.

Trying the --squash option of the latest ghmerge on #11134 yesterday revealed that its error handling still deserved some improvements, which in turn motivated a further tweak also on addrev. These are done in the two most recent commits.

Any further comments or requests for changes?
Else would be nice if this can approved such that it becomes directly usable.

Thanks @t8m and @paulidale for your swift approvals.

Can an OMC member please now do the merge?
You may already take advantage of the new ghmerge to merge itself 😉, e.g., like this:

ghmerge --tools 59 @t8m paulidale

I've merged it but I don't think I used the new ghmerge. I would have used the new versions of things it calls.

I've merged it but I don't think I used the new ghmerge. I would have used the new versions of things it calls.

Well now after this merge the ghmerge is becoming really useful tool.

Well now after this merge the ghmerge is becoming really useful tool.

Very pleased to hear!