From: Junio C Hamano <gitster@pobox.com>
To: Jay Soffian <jaysoffian@gmail.com>
Cc: git@vger.kernel.org, Marc Branchaud <marcnarc@xiplink.com>,
Johannes Schindelin <Johannes.Schindelin@gmx.de>,
Johannes Sixt <j.sixt@viscovery.net>
Subject: Re: [PATCH 1/4] remote: minor code cleanups in preparation for changing "show" output
Date: Thu, 19 Feb 2009 23:19:45 -0800 [thread overview]
Message-ID: <7vbpsxh8xa.fsf@gitster.siamese.dyndns.org> (raw)
In-Reply-To: <1235020471-59982-2-git-send-email-jaysoffian@gmail.com> (Jay Soffian's message of "Thu, 19 Feb 2009 00:14:28 -0500")
Jay Soffian <jaysoffian@gmail.com> writes:
> * Rename char *remote to remote_name to distinguish it clearly from the
> struct remote pointer, also named remote.
>
> * There is no need to call sort_string_list() on branch_list, as its
> items are added to it via string_list_insert() which maintains its
> order.
>
> * Sort states->new and states->tracked so that we can use binary search
> string_list_has_string() on them instead of less efficient linear
> unsorted_string_list_has_string. This alters the output of "remote
> show" slightly, so update the tests to match.
>
> * Simplify get_ref_states(); nothing is using the pointer to states that
> is being copied into util.
>
> * Have get_remote_ref_states() populate states->tracked even when it is
> not querying the remote so that this need not be done by the caller.
This does too many things in a single patch.
Ideally this would have been four patches for reviewability:
- one "trivial and obviously correct bits" (s/remote/remote_name/ and
removal of sort_string_list(&branch_list)) patch;
- the change for states->{new,tracked}, should stand on its own; I think
the reordering of the output should be described much better and
defended independently. "Earlier it was sorted by this order, which
did not make sense for such and such reasons; this fixes the logic to
sort the list by the name of the tracked branch, which makes it easier
to read", or something like that.
- change to the states->tracked population rule; and
- get_ref_states() to lose the util bit.
It probably is Ok to squash the last two, though.
next prev parent reply other threads:[~2009-02-20 7:21 UTC|newest]
Thread overview: 19+ messages / expand[flat|nested] mbox.gz Atom feed top
2009-02-19 5:14 [PATCH 0/4] Improve "remote show" output Jay Soffian
2009-02-19 5:14 ` [PATCH 1/4] remote: minor code cleanups in preparation for changing "show" output Jay Soffian
2009-02-19 5:14 ` [PATCH 2/4] remote: move append_ref_to_tracked_list to get rid of prototype Jay Soffian
2009-02-19 5:14 ` [PATCH 3/4] string-list: add for_each_string_list() Jay Soffian
2009-02-19 5:14 ` [PATCH 4/4] remote: new show output style Jay Soffian
2009-02-19 16:03 ` Marc Branchaud
2009-02-19 16:16 ` Sverre Rabbelier
2009-02-19 16:31 ` Marc Branchaud
2009-02-19 16:33 ` Sverre Rabbelier
2009-02-19 16:17 ` Rostislav Svoboda
2009-02-19 17:57 ` Jay Soffian
2009-02-19 17:59 ` Jay Soffian
2009-02-19 18:58 ` Julian Phillips
2009-02-20 22:34 ` Marc Branchaud
2009-02-20 22:55 ` Jay Soffian
2009-02-19 19:29 ` Johannes Sixt
2009-02-19 19:51 ` Jay Soffian
2009-02-20 7:19 ` Junio C Hamano [this message]
2009-02-20 10:50 ` [PATCH 1/4] remote: minor code cleanups in preparation for changing "show" output Jay Soffian
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=7vbpsxh8xa.fsf@gitster.siamese.dyndns.org \
--to=gitster@pobox.com \
--cc=Johannes.Schindelin@gmx.de \
--cc=git@vger.kernel.org \
--cc=j.sixt@viscovery.net \
--cc=jaysoffian@gmail.com \
--cc=marcnarc@xiplink.com \
/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.
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 NNTP newsgroup(s).