[v5,0/5] branch: operations on orphan branches

Message

Rubén Justo March 26, 2023, 10:19 p.m. UTC

The initial and main intention in this series is to avoid some confusing
errors operating with orphan branches, when working with worktrees.

It's been a while since the previous iteration, sorry for that.  Here is
a quick overview of the previous iterations:

In v2 a change was introduced, to avoid some unnecessary worktrees
traversals, which implies disk access.

In v3 and v4 some refactorings were done, such as making some names
more meaningful and, mainly, inlining some helper functions in its only
one caller.  No functional changes were expected.  However, those
refactorings introduced some undesired behavior changes; some were noted
in the round of reviews, some others during the preparation of this new
iteration.

In this iteration, v5, I've reverted some of those major refactorings,
mainly the inlining, because it ended up introducing unnecessary
complexity for minimal benefit in this series.  Maybe those refactorings
make more sense in future series.

A new test has been introduced, in 1/5, to notice if a behavior change
similar to that observed in v4, is reintroduced in the future.

Other than that, no functional changes are expected from v2.

Rubén Justo (5):
  branch: test for failures while renaming branches
  branch: use get_worktrees() in copy_or_rename_branch()
  branch: description for orphan branch errors
  branch: rename orphan branches in any worktree
  branch: avoid unnecessary worktrees traversals

 branch.c               | 27 ----------------
 branch.h               |  8 -----
 builtin/branch.c       | 71 ++++++++++++++++++++++++++++++++++--------
 t/t3200-branch.sh      | 29 +++++++++++++++++
 t/t3202-show-branch.sh | 18 +++++++++++
 5 files changed, 105 insertions(+), 48 deletions(-)

Comments

Junio C Hamano March 27, 2023, 7:49 p.m. UTC | #1

Rubén Justo <[e-mail]> writes:

> In this iteration, v5, I've reverted some of those major refactorings,
> mainly the inlining, because it ended up introducing unnecessary
> complexity for minimal benefit in this series.  Maybe those refactorings
> make more sense in future series.

The previous one has been cooking in 'next' already for some time
(even before the feature freeze for 2.40), so we'd usually do not
take such a rewrite and rather go with incremental refinements, but
let's discard it out of 'next' and queue this intereation instead.

We're probably overdue to rewind and rebuild 'next' this round
anyway.

Thanks for an updated version.  Will queue.

Junio C Hamano May 1, 2023, 10:19 p.m. UTC | #2

Rubén Justo <[e-mail]> writes:

> The initial and main intention in this series is to avoid some confusing
> errors operating with orphan branches, when working with worktrees.
> ...
> In this iteration, v5, I've reverted some of those major refactorings,
> mainly the inlining, because it ended up introducing unnecessary
> complexity for minimal benefit in this series.  Maybe those refactorings
> make more sense in future series.
>
> A new test has been introduced, in 1/5, to notice if a behavior change
> similar to that observed in v4, is reintroduced in the future.
>
> Other than that, no functional changes are expected from v2.

This has not seen any activities since it was posted; presumably the
issues raised during the review of the previous round have all been
addressed?

Is everybody happy to see us declare victory and merge it down to
'next'?  I see everybody who commented on earlier iterations of the
series are CC'ed, so let's hear from them (and others who may be
interested).

Thanks.