From: greened@obbligato.org (David A. Greene)
To: Junio C Hamano <gitster@pobox.com>
Cc: git@vger.kernel.org, techlivezheng@gmail.com,
alex.crezoff@gmail.com, davvid@gmail.com,
cbailey32@bloomberg.net, danny0838@gmail.com, prohaska@zib.de,
th.acker@arcor.de, sschuberth@gmail.com, peff@peff.net,
gitter.spiros@gmail.com, nod.helm@gmail.com
Subject: Re: [PATCH] contrib/subtree: Remove --annotate
Date: Tue, 05 Jan 2016 15:35:23 -0600 [thread overview]
Message-ID: <87oaczwvz8.fsf@waller.obbligato.org> (raw)
In-Reply-To: <xmqqsi2cj5hu.fsf@gitster.mtv.corp.google.com> (Junio C. Hamano's message of "Tue, 05 Jan 2016 09:33:33 -0800")
Junio C Hamano <gitster@pobox.com> writes:
> David Greene <greened@obbligato.org> writes:
>
>> From: "David A. Greene" <greened@obbligato.org>
>>
>> Remove --annotate. This obviates the need for an --unannotate
>> command, which is both an obvious addition and difficult to define
>> due to the numerous ways one might want to specify how to edit
>> commit messages. git has other tools more suited to rewriting
>> commit messages and it's easy enough to use them after a subtree
>> split. Such tools include filter-branch, rebase -i and
>> commit --amend.
>
> I do not think that "there are other ways to do this" is a good
> justification for removing a feature, unless it can be shown that
> nobody is using it, of course.
I thought you might say that. :)
I honestly don't know how much it's used. Obviously someone uses it
because we got a request a couple of years ago for an --unannotate
option and the ensuing discussion made it clear that that's not a
trivial thing.
The original author is not active anymore so I don't even know why it
was added in the first place. I don't know how to get data about usage.
I'm in the process of getting git-subtree into shape so it can move out
of contrib into the main area. Is there a policy for interface changes
to things in contrib?
There are a few other things I'm working on that will involve slight
semantic changes. I was planning to do those similarly to how push
changed with git 2.0. Make the current behavior default, emit a warning
and switch the default after a few releases. This is all being done to
make git-subtree faster, take advanted of new git features since it was
opriginally written and generally make it more solid and predictable. I
could do the deprecate/remote thing with --annotate if that sounds
better to you.
My thinking on this change runs as follows:
- --annotate isn't as powerful/flexible as other git commit message
rewrite tools.
- Its obvious pair feature --unannotate isn't trivial to do -- people
didn't even agree on what it *should* do. It won't be added any
time soon, if at all, so don't advertise something that naturally
leads people to request it.
- We really shouldn't lie about the state of this. --annotate feels
tacked-on and incomplete. It would be best to not have it at all if
we can't do it right.
- Better to make the change now before moving out of contrib.
If you really don't want to get rid of this, I guess that's ok but my
preference as maintainer is to reduce the feature set to those things
people seem to actually regularly use (according to my very unscientific
Google searches) and add features as requested/evaluated. --annotate
isn't a huge maintenance burdern but some of those other changes I
mentioned do in fact significantly reduce the maintenance burden of
git-subtree. I hope I will have some leeway with those, even if they
change semantics slightly.
>> @@ -319,7 +315,7 @@ copy_commit()
>> GIT_COMMITTER_NAME \
>> GIT_COMMITTER_EMAIL \
>> GIT_COMMITTER_DATE
>> - (printf "%s" "$annotate"; cat ) |
>> + (echo -n ""; cat ) |
>
> I can see that by changing "printf something" with 'echo -n ""', you
> are making it clear that we are stopping to add that something to
> the pipeline, but (1) I think the intended effect of running 'echo
> -n' on an empty string is to do nothing, and (2) 'echo -n' is not
> portable [*1*], so this leaves a puzzling code that makes future
> readers scratch their heads.
>
> I wonder why this cannot be simply the removal of the entire line,
> making the resulting implementation more like this:
>
> git log -1 --pretty=format:... "$1" |
> (
> read ... various variables ...
> export ... various variables ...
> - (printf "%s" "$annotate"; cat ) |
> git commit-tree "$2" $3 # reads the rest of stdin
> ) || die "cannot copy"
>
> That is, just feed the remainder of what is coming directly to the
> command?
That makes sense. Thanks.
-David
next prev parent reply other threads:[~2016-01-05 21:35 UTC|newest]
Thread overview: 11+ messages / expand[flat|nested] mbox.gz Atom feed top
2016-01-05 3:05 [PATCH v2] Remote subtree split --annotate David Greene
2016-01-05 3:05 ` [PATCH] contrib/subtree: Remove --annotate David Greene
2016-01-05 17:33 ` Junio C Hamano
2016-01-05 21:35 ` David A. Greene [this message]
2016-01-15 18:54 ` Junio C Hamano
2016-01-17 23:30 ` David A. Greene
2016-01-18 1:29 ` Junio C Hamano
2016-06-28 11:10 ` David A. Greene
-- strict thread matches above, loose matches on Subject: below --
2016-01-02 20:36 Remote subtree split --annotate David Greene
2016-01-02 20:36 ` [PATCH] contrib/subtree: Remove --annotate David Greene
2016-01-03 9:06 ` Sebastian Schuberth
2016-01-04 15:53 ` greened
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
List information: http://vger.kernel.org/majordomo-info.html
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=87oaczwvz8.fsf@waller.obbligato.org \
--to=greened@obbligato.org \
--cc=alex.crezoff@gmail.com \
--cc=cbailey32@bloomberg.net \
--cc=danny0838@gmail.com \
--cc=davvid@gmail.com \
--cc=git@vger.kernel.org \
--cc=gitster@pobox.com \
--cc=gitter.spiros@gmail.com \
--cc=nod.helm@gmail.com \
--cc=peff@peff.net \
--cc=prohaska@zib.de \
--cc=sschuberth@gmail.com \
--cc=techlivezheng@gmail.com \
--cc=th.acker@arcor.de \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
Code repositories for project(s) associated with this public inbox
https://80x24.org/mirrors/git.git
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).