On 18/11/30 07:19, Catalin Marinas wrote:
> On Fri, 30 Nov 2018 at 12:26, Ismael Bouya
> <
ismael....@normalesup.org> wrote:
> > > StGit has a "commit" command to "freeze" the state of a branch and
> > > avoid inadvertently changing the sha1 of the patches to be published
> > > (assuming that the patches you don't want are popped from the stack).
> > > Ideally you'd send the PR with a (signed) tag that points to the
> > > stable sha1. This way, after the PR, you can continue to work on that
> > > branch, push additional patches on top and even publish them as long
> > > as the tag in the PR is not changed (and you don't "uncommit" any
> > > patches).
> >
> > But how do you handle needed changes with that workflow? (usually when
> > you do a PR, you expect a review and possibly some changes to do to that
> > PR)
> > With "bare git" I commit --amend (/rebase) and repush and it updates on
> > github/gitlab side. With a tag, this cannot work.
The workflow Catalin described involves pushing new commits for any
changes needed to the PR after it is initially published.
Another workflow can be used if it is acceptable to change the commits
associated with a published PR instead of just adding new commits. Given
a series with patches A-F where A, B, and C are the patches for the PR,
you can simply apply and push the patches associated with your PR:
stg goto C
git push origin master
A PR can then be created based on your master branch. If you have to
make changes to any of your patches (i.e. due to comments on your PR):
stg goto B
<edit files>
stg refresh
stg goto C
git push origin +master
Rinse and repeat as necessary.
While the PR is pending, you can continue working on other patches in
your queue (D, E, F, ...).
Once the PR is merged into the upstream repo, you can cleanup your
patches:
stg goto C
stg pull --merged upstream
Patches A, B, and C should now be empty. Verify with:
stg series -e
Finally, remove all the now-empty patches with:
stg clean
There are a couple of things about the above workflow that do not meet
your goals:
1. PR is on master, not a named branch.
2. Only works for one PR at a time.
However, this workflow can be good in cases where only having one
outstanding PR at a time is acceptable.
> > Here is a scenario that comes to mind of what I’d like to do:
> >
> > I am on the branch "master", with a bunch of patches on top of it,
> > I want to push patches A B and E to a new PR:
> > - stg create-pull-request --branch=cool_feature A B E
> > - git push origin cool_feature
> > - (edit patch A with the maintainer feedback, the commands to do that
> > already exist in stgit)
> > - stg update-pull-request --branch=cool_feature
> > - git push origin +cool_feature
> > - (maybe some comment to "close" the PR once merged or refused)
>
> There is also a "stg sync" command. You can create a PR branch where
> you cherry-pick some of the patches but continue to work on your
> development branch. Before the PR, you switch to the PR branch and run
> a "stg sync -a" which, in theory, should synchronise the patches with
> the corresponding ones in your master branch. Now, I don't remember
> when I last used this command, I even forgot it existed ;).
I had never successfully used `stg sync`. I just did a little
experimentation, but when I initialized my PR branch by `stg pick`ing
patches from master, I ended up with merge conflicts even in trivial
cases where I only modified the patch on master. Also, it doesn't seem
like `stg sync` synchronizes the commit messages.
> We could probably do this as well with an extension to "sync" to be
> able to update patches in a different branch ("--to=" option) without
> switching the current one. We could also replace the more complicated
> 3-way merge of "sync" with a re-write of the patches on the --to=
> branch since handling a 3-way merge outside the current index is
> problematic anyway.
+1 for this idea. The 3-way merge behavior does not really fit the use
case where canonical patches are maintained in a master series and
farmed-out to other branches for creating PR's. Maybe a `--replace`
option as well to tell `stg sync` to do simple replacement of patches on
the target branch instead of the 3-way merge.
stg branch --create cool_feature
stg pick -B master A B E
stg branch master
git push origin cool_feature
<create PR, get feedback>
stg goto A
<edit>
stg refresh
stg sync --replace --to=cool_feature A B E
git push origin +cool_feature
> But I'm open to having a completely different command for preparing
> PRs. "commit" has been sufficient for my workflow so far.
Agreed. StGit features that help with managing GitHub-style PR's would
be welcome. I'm open to both new subcommands and improvements to
existing subcommands like `sync`, `publish`, `branch`, and `pick`.
Pete