sapling icon indicating copy to clipboard operation
sapling copied to clipboard

add --force-with-lease for sl push

Open suiwombat opened this issue 3 years ago • 1 comments

Hey guys, sl has --force, but it doesn't have --force-with-lease. It'd be great to have that as a flag for sl push. thanks!!

client version: Sapling 0.2.20230228-144002

git-push help: git-push

sl push [OPTION]... [--to BOOKMARK] [-r REV]... [DEST]

push commits to the specified destination

    Push commits from the local repository to the specified destination.

    Use "-t/--to" to specify the remote bookmark. For Git repos, remote
    bookmarks correspond to Git branches.

    To add a named remote destination, see 'sl path --add'.

    "-r/--rev" specifies the commit(s) (including ancestors) to push to the
    remote repository. Defaults to the current commit.

    Add "--create" to create the remote bookmark if it doesn't already exist.

    The "-f/--force" flag allows non-fast-forward pushes.

    If DESTINATION is omitted, the default path will be used. See 'sl help
    urls' and 'sl help path' for more information.

    Returns 0 on success.

Options ([+] can be repeated):

 -f --force           force push
 -r --rev REV [+]     a commit to push
 -t --to BOOKMARK     push commits to this bookmark
 -d --delete BOOKMARK delete remote bookmark
    --create          create a new remote bookmark

suiwombat avatar Mar 17 '23 15:03 suiwombat

That's a good idea, but what should happen if a newer commit has been pushed to the PR?

sl pr currently wouldn't have any way of dealing with it, so you'd have to force push your commit to the PR branch which then breaks the special GitHub integration. To handle this, we'd also need a sl pr submit --force flag.

However, from my use of Sapling force-with-lease isn't needed. On the codebases I contribute to the author normally owns the PR, and it's unusual for someone else to make commits to it. There are only two times this happens for me:

  1. sometimes a reviewer will merge in main to my PR
  • In this case, I don't want sl pr s to fail. In fact, normally I rely on this behavior because I have an alias for sl pull && sl rebase -b 'draft()' -d main && sl pr s
  1. suggestions on PRs
  • this is a just a fundamental incompatibility with GitHub's and Sapling model for patches. I don't see how GitHub PR suggestions will ever not be broken with Sapling. Two possible workarounds (that has nothing to do with this issue) i. support suggestions in ReviewStack ii. sl pr subcommand to review and accept suggestions on PRs, which will absorb suggestions and resubmit the diff

vegerot avatar Mar 20 '23 22:03 vegerot