Re: [PATCH 1/4] remote: minor code cleanups in preparation for changing "show" output

Jay Soffian [off-list ref] 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.

On Fri, Feb 20, 2009 at 2:19 AM, Junio C Hamano [off-list ref] wrote:

This does too many things in a single patch.

Ideally this would have been four patches for reviewability:

Okay. I'm re-doing 4/4 anyway, so I'll just re-do the series.

j.