[Commented On] D11038: amend: make `hg amend -r` fold temporary commit into target commit

[marmoute (Pierre-Yves David)]

marmoute added a comment.


  Not much feedback here but for the previous one of "why are we going through the full trouble of doing cross extensions delegation instead of using rewriteutil function instead?

INLINE COMMENTS

> amend.py:263-267
> +                rev=[
> +                    revsetlang.formatspec(
> +                        b'%d + %d', target_ctx.rev(), rebased_temp_ctx.rev()
> +                    )
> +                ],

This would probably win to be prepared outside of the call to help readability.

(also, this seems weird and increase my questioning about why we are using rebase directly instead of a more appropriate API).

> amend.py:268-270
> +                dest=revsetlang.formatspec(
> +                    b'%ld', [pctx.rev() for pctx in target_ctx.parents()]
> +                ),

This would probably win to be prepared outside of the call to help readability.

(also, this seems weird and increase my questioning about why we are using rebase directly instead of a more appropriate API).

> amend.py:275-276
> +
> +    if ret:
> +        raise error.Abort(_(b'failed to fold temporary commit'))
> +

Is there any cases where this could effectively fails ?

REPOSITORY
  rHG Mercurial

CHANGES SINCE LAST ACTION
  https://phab.mercurial-scm.org/D11038/new/

REVISION DETAIL
  https://phab.mercurial-scm.org/D11038

To: martinvonz, #hg-reviewers, Alphare
Cc: marmoute, mercurial-patches
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.mercurial-scm.org/pipermail/mercurial-patches/attachments/20210716/e2b6fb04/attachment-0002.html>