From: "David Kågedal" <davidk@lysator•liu.se>
To: "Karl Hasselström" <kha@treskal•com>
Cc: git@vger•kernel.org, catalin marinas <catalin.marinas@gmail•com>
Subject: Re: [StGit PATCH] Add a --tree flag to stg push
Date: Tue, 19 May 2009 09:50:26 +0200 [thread overview]
Message-ID: <87d4a5fs59.fsf@krank.kagedal.org> (raw)
In-Reply-To: <20090519072512.GA8451@diana.vm.bytemark.co.uk> ("Karl Hasselström"'s message of "Tue\, 19 May 2009 09\:25\:12 +0200")
Karl Hasselström <kha@treskal•com> writes:
> On 2009-05-18 16:50:18 +0200, David Kågedal wrote:
>
>> This scratches a long-time itch for me. The typical use case is when
>> you want to break up a larg patch inte smaller ones. You back out
>> the orignal patch, apply a small set of changes from it and then
>> push the patch back again. But then you don't want to do a merge,
>> with the possibility of conflict. You simply want to restore to the
>> tree that the patch had before so you can see what's left to create
>> cleaned-up patches of. The command "stg push --tree" does just that.
>
> Thanks!
>
> There's no sign-off.
I counted on getting comments, so it's not finished yet...
>> The naming of flags and functions isn't very obvious, and
>> suggestions for improvements are welcome.
>
> --set-tree maybe?
Probably better. But perhaps there is a way to not have to talk about
"trees" at all?
>> t/t1207-push-tree.sh | 64 ++++++++++++++++++++++++++++++++++++++++++++++
>
> A test! Very good.
>
>> + opt('--tree', action = 'store_true',
>> + short = 'Push the patch with the original tree')
>
> This probably deserves a long description as well. (That most existing
> options lack them is unfortunate---the support for long descriptions
> was added rather recently.)
I didn't look putside push.py, and just followed the pattern
there. But a long description sounds like a good idea. It won't be
obvious what this does with just a short one.
>> + if any(getattr(cd, a) != getattr(orig_cd, a) for a in
>> + ['parent', 'tree', 'author', 'message']):
>> + comm = self.__stack.repository.commit(cd)
>> + self.head = comm
>> + else:
>> + comm = None
>> + s = ' (unmodified)'
>
> Shouldn't self.head be set in both cases?
I guess so. I'm a bit unsure about the correctness of that whole
function.
>> +# Copyright (c) 2006 David Kågedal
>
> Been sitting on this patch long? :-)
Copy/paste error.
>> +# don't need this repo, but better not drop it, see t1100
>> +#rm -rf .git
>> +
>> +# Need a repo to clone
>> +test_create_repo foo
>
> Umm, your test doesn't seem to depend on using this separate repo
> instead of the default one.
Call it copy/paste programming or cargo cult programming. I will clean
up.
--
David Kågedal
next prev parent reply other threads:[~2009-05-19 7:50 UTC|newest]
Thread overview: 6+ messages / expand[flat|nested] mbox.gz Atom feed top
2009-05-18 14:50 [StGit PATCH] Add a --tree flag to stg push David Kågedal
2009-05-19 7:25 ` Karl Hasselström
2009-05-19 7:50 ` David Kågedal [this message]
2009-05-19 8:03 ` Karl Hasselström
2009-05-19 9:35 ` [StGit PATCH v2] Add a --set-tree " David Kågedal
2009-05-19 10:27 ` Karl Hasselström
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=87d4a5fs59.fsf@krank.kagedal.org \
--to=davidk@lysator$(echo .)liu.se \
--cc=catalin.marinas@gmail$(echo .)com \
--cc=git@vger$(echo .)kernel.org \
--cc=kha@treskal$(echo .)com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox