Re: [PATCH v2 0/9] Introduce publish tracking branch
Matthieu Moy wrote: > Felipe Contreras writes: > > > My patch series only affects push.default=simple, perhaps you have a > > different configuration. > > Good catch. I have push.default=upstream (essentially for compatibility > with old Git versions, I'd prefer simple actually). > > > Maybe we want the publish branch to override any push.default, so: > > Not sure actually. If a user says "push.default=upstream", it seems > weird to push to something other than upstream indeed. What's clear to > me is that your patch in its current form clearly makes "simple" a much > better default than "upstream" (good news, it it the default!). As you said in another email; that's just the default. If the user explicitely told Git to use certain branch (git push -p), Git should use that branch. > That said, the advice given by "git status" is clearly wrong: > > > > $ git status > > > On branch master > > > Your branch is ahead of 'origin/new' by 4 commits. > > > (use "git push" to publish your local commits) > > It should say (use "git push origin new" to publish your local commits) > with push.default=upstream and the current behavior of the patch. > > Perhaps argumentless "git push" could warn when push.default=upstream > and branch..publish is configured, I'm not sure. > > Sorry, more questions and "I'm not sure" than actual suggestion :-(. I believe in v3 of the patch series "git push" will actually do it correctly. Cheers. -- Felipe Contreras -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 0/9] Introduce publish tracking branch
Felipe Contreras writes: > My patch series only affects push.default=simple, perhaps you have a > different configuration. Good catch. I have push.default=upstream (essentially for compatibility with old Git versions, I'd prefer simple actually). > Maybe we want the publish branch to override any push.default, so: Not sure actually. If a user says "push.default=upstream", it seems weird to push to something other than upstream indeed. What's clear to me is that your patch in its current form clearly makes "simple" a much better default than "upstream" (good news, it it the default!). That said, the advice given by "git status" is clearly wrong: > > $ git status > > On branch master > > Your branch is ahead of 'origin/new' by 4 commits. > > (use "git push" to publish your local commits) It should say (use "git push origin new" to publish your local commits) with push.default=upstream and the current behavior of the patch. Perhaps argumentless "git push" could warn when push.default=upstream and branch..publish is configured, I'm not sure. Sorry, more questions and "I'm not sure" than actual suggestion :-(. -- Matthieu Moy http://www-verimag.imag.fr/~moy/ -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 0/9] Introduce publish tracking branch
Matthieu Moy wrote:
> Felipe Contreras writes:
>
> > As it has been discussed before, our support for triangular workflows is
> > lacking, and the following patch series aims to improve that situation.
>
> I'm not a heavy user of triangular workflow, so I'm not in the best
> position to comment (and I have no time for a real review, sorry).
>
> On overall, I do like the change. I played a bit with it, and do not
> understand what "git push" does:
>
> $ git status
> On branch master
> Your branch is ahead of 'origin/new' by 4 commits.
> (use "git push" to publish your local commits)
>
> => OK, it's using the publish branch to tell me whether I should push.
>
> $ git push -v
> Pushing to /tmp/git
> To /tmp/git
>= [up to date] master -> master
> updating local tracking ref 'refs/remotes/origin/master'
> Everything up-to-date
>
> => Err, it still pushes to the upstream branch ... Wasn't that the point
> of the change to push to publish? Did I do something wrong?
My patch series only affects push.default=simple, perhaps you have a
different configuration.
Maybe we want the publish branch to override any push.default, so:
--- a/builtin/push.c
+++ b/builtin/push.c
@@ -195,11 +195,7 @@ static void setup_push_current(struct remote *remote,
struct branch *branch)
static void setup_push_simple(struct remote *remote, struct branch *branch,
int triangular)
{
- if (branch->push_name) {
- struct strbuf refspec = STRBUF_INIT;
- strbuf_addf(&refspec, "%s:%s", branch->name, branch->push_name);
- add_refspec(refspec.buf);
- } else if (triangular) {
+ if (triangular) {
setup_push_current(remote, branch);
} else {
setup_push_upstream(remote, branch, triangular);
@@ -260,8 +256,16 @@ static struct branch *get_current_branch(struct remote
*remote)
static void setup_default_push_refspecs(struct remote *remote)
{
+ struct branch *branch = branch_get(NULL);
int triangular = is_workflow_triangular(remote);
+ if (branch && branch->push_name) {
+ struct strbuf refspec = STRBUF_INIT;
+ strbuf_addf(&refspec, "%s:%s", branch->name, branch->push_name);
+ add_refspec(refspec.buf);
+ return;
+ }
+
switch (push_default) {
default:
case PUSH_DEFAULT_UNSPECIFIED:
--
Felipe Contreras
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 0/9] Introduce publish tracking branch
Felipe Contreras writes:
> As it has been discussed before, our support for triangular workflows is
> lacking, and the following patch series aims to improve that situation.
I'm not a heavy user of triangular workflow, so I'm not in the best
position to comment (and I have no time for a real review, sorry).
On overall, I do like the change. I played a bit with it, and do not
understand what "git push" does:
$ git status
On branch master
Your branch is ahead of 'origin/new' by 4 commits.
(use "git push" to publish your local commits)
=> OK, it's using the publish branch to tell me whether I should push.
$ git push -v
Pushing to /tmp/git
To /tmp/git
= [up to date] master -> master
updating local tracking ref 'refs/remotes/origin/master'
Everything up-to-date
=> Err, it still pushes to the upstream branch ... Wasn't that the point
of the change to push to publish? Did I do something wrong?
Your series lacks documentation of branch..* in
Documentation/config.txt.
It seems strange to me to name the config variables "branch..push"
and "branch..pushremote" and call the same thing "@{publish}"
elsewhere. We're already not consistant with @{upstream} corresponding
to branch..merge, but I do not consider it as a good reason to
introduce one more inconsistancy.
--
Matthieu Moy
http://www-verimag.imag.fr/~moy/
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 0/9] Introduce publish tracking branch
A handful of minimum tweaks [*1*] here and there were necessary in order to queue the series on 'pu', but to me, the feature looked like quite a straight-forward addition. I'd be dropping the jk/branch-at-publish-rebased from 'pu', at least tentatively, as that one was primarily Peff giving Ram a base to build on top to achieve essentially the same thing as this series does. I didn't bother to check if this series could have reused some from that series (primarily because I was short of time, had to take the work laptop to service, etc. etc.) before doing so. I didn't think too deeply about the workflow ramifications of this series brings in, either---that is left for the reviewers Cc'ed on the patches. [Footnote] *1* Things like these: - the context in builtin/push.c has already changed at the tip of 'master' (we already pretend to be Git 2.0) and the patch text needed to be adjusted. - an instance of cast to "(char*)" fixed to "(char *)". - t/t5529 is already used by other topics, renaming the new test to t/t5534-push-publish.sh. - !prefixcmp() is already removed for Git 2.0, replacing its use with starts_with(). -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 0/9] Introduce publish tracking branch
As it has been discussed before, our support for triangular workflows is
lacking, and the following patch series aims to improve that situation.
We have the concept of upstream branch (e.g. 'origin/master') which is to where
our topic branches eventually should be merged to, so it makes sense that
'git rebase' uses that as the destination, but most people would not push to
such upstream branch, they would push to a publish branch
(e.g. 'github/feature-a'). We could set our upstream to the place we push, and
'git push' would be able to use that as default, and 'git branch --vv' would
show how ahead/behind we are in comparisson to that branch, but then
'git rebase' (or 'git merge') would be using the wrong branch.
This patch series adds:
1) git push --set-publish
2) git branch --set-publish
3) git branch -vv # uses and shows the publish branch when configured
4) @{publish} and @{p} marks
5) branch.$name.{push,pushremote} configurations
After this, it becomes much easier to track branches in a triangular workflow.
The publish branch is used instead of the upstream branch for tracking
information in 'git branch --vv' and 'git status' if present, otherwise there
are no changes (upstream is used).
master e230c56 [origin/master, gh/master] Git 1.8.4
* fc/publish 0a105fd [master, gh/fc/publish: ahead 1] branch: display
publish branch
fc/branch/fast 177dcad [master, gh/fc/branch/fast] branch: reorganize
verbose options
fc/trivial f289b9a [master: ahead 7] branch: trivial style fix
fc/leaksd101af4 [master: ahead 2] read-cache: plug a possible leak
stable e230c56 Git 1.8.4
Changes since v1:
* Added @{publish} and @{p} marks
Felipe Contreras (9):
push: trivial reorganization
Add concept of 'publish' branch
branch: allow configuring the publish branch
t: branch add publish branch tests
push: add --set-publish option
branch: display publish branch
sha1_name: cleanup interpret_branch_name()
sha1_name: simplify track finding
sha1_name: add support for @{publish} marks
Documentation/git-branch.txt | 11 +++
Documentation/git-push.txt | 9 +-
Documentation/revisions.txt | 4 +++
branch.c | 43 +
branch.h | 2 ++
builtin/branch.c | 74 ++
builtin/push.c | 52 +++---
remote.c | 34
remote.h | 4 +++
sha1_name.c | 62 ++--
t/t1508-at-combinations.sh | 5 +++
t/t3200-branch.sh| 76
t/t5529-push-publish.sh | 70
t/t6040-tracking-info.sh | 5 +--
transport.c | 28 ++--
transport.h | 1 +
16 files changed, 415 insertions(+), 65 deletions(-)
create mode 100755 t/t5529-push-publish.sh
--
1.9.1+fc1
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
