In trying to understand what happened, I took a closer look, and added
a comment to that ticket.
Here's what I understand is a rough timeline of what happened:
1. Ed created a PR to merge 2.1 forward into main (believing he was
creating a PR from his fork)
2. Ed reached out to me to ask about how to merge 2.1 forward, since
it's not a frequent enough thing for him to be super familiar with the
process (this may have occurred before item 1, but I didn't respond
before item 1)
3. I saw the main branch was not up-to-date with the latest additions
to 2.1, so I merged 2.1 into main from the command-line, which caused
GitHub to close the PR and suggest deleting the originating branch,
since all commits were merged into main
4. Ed clicked the Delete button, thinking GitHub was offering to
delete a branch in his fork
5. Chris S. restored the 2.1 branch at the commit that was deleted
So, I think GitHub is partially to blame here for making the UI not
very intuitive (it really needs to show the originating branch
location more clearly, and shouldn't offer the delete button for local
maintenance branches... but not sure how it would know). We could add
branch protections, but would rely on GitHub to enforce, and this
would be a hassle to customize our preferences with INFRA, and would
be another hassle when we wanted to clean up old branches later. I'm
not sure that's worth the trouble.
Ultimately, though, I think step 1 isn't necessary. If I hadn't merged
from the command-line, and we merged from the UI instead using our
normal squash, then it would have created a duplicate commit, not
linked to the one in the first branch. This is not what we want. So,
creating the PR for a merge forward isn't necessary. In this case, it
was a conflict-free merge forward from the command-line. Most merges
forward are conflict free or require trivial conflict resolution and
don't need to be re-reviewed. In the case where the merge forward will
be complex with conflicts, I think it might be better to either start
with the main branch, then create a separate PR to backport to the old
branch, or just merge forward using `-s ours` to create a NOOP merge
that merges the history, but doesn't change anything in the main
branch, and then create a new PR to re-apply the changes tailored to
the newer branch. I'd prefer the "backport" method instead of the
"reapply" method, personally, but either would work. In any case, that
only matters for merges forward with complex merge conflict
resolution. For trivial conflict resolution, this isn't necessary at
all.
Anyway, I hope this helps people when merging forward in future. In
general, it's better to do it from the command-line, and not using the
GitHub UI.
Thanks,
Christopher
On Mon, Jan 23, 2023 at 1:48 PM dev1 wrote:
>
> The 2.1 branch on Accumulo GiHub repo was inadvertently deleted (Friday 1/20)
> and then was restored Sat (1/21). The activity occurred due to
> https://github.com/apache/accumulo/pull/3165. Posting this here in case
> anyone was impacted and to preserve a record in the archive.
>
> Ed Coleman
>
