Re: [PATCH 0/6] repack_without_refs(): convert to string_list

On Fri, Nov 21, 2014 at 10:00 AM, Junio C Hamano [off-list ref] wrote:

Michael Haggerty [off-list ref] writes:

quoted

I don't think that those iterations changed anything substantial that
overlaps with my version, but TBH it's such a pain in the ass working
with patches in email that I don't think I'll go to the effort of
checking for sure unless somebody shows interest in actually using my
version.

Sorry for being grumpy today :-(

Sorry for causing the grumpyness.
I have compared the versions, and they do look pretty similar.
In refs.{c,h} we're just talking about variable names and comments,
that are different.

In remote.c prune_remote however we did have slight differences,
* early exit vs a large body below an if
* your approach seems more elegant to me as you seem to know what you're doing:
       for_each_string_list_item(item, &states.stale)
               string_list_append(&refs_to_prune, item->util);
 instead of
       for (i = 0; i < states.stale.nr; i++)
               string_list_append(&delete_refs, states.stale.items[i].util);
* You do not have a sort_string_list at the end before warn_dangling_symrefs,
   but you explained that it is not necessary.

On my continued journey on this mailing list I'll try to follow your
example and write lots of
small easy to review patches, as they are indeed way easier to follow.

However as Junio mentioned, we get other problems having too small changes.
In the review for the [PATCH v3 00/14] ref-transactions-reflog series you said:

I was reviewing this patch series (I left some comments in Gerrit about
the first few patches) when I realized that I'm having trouble
understanding the big picture of where you want to go with this.

Maybe that was just my fault, not having stated the intentions in
the cover letter explicit enough. But having many patches will also not help
on presenting the big picture easily.

Thanks for bearing with me,
Stefan

Is the above meant as a grumpy rant to be ignored, or as a
discussion starter to improve the colaboration to allow people to
work better together instead of stepping on each other's patches?

FWIW, I liked your rationale for "many smaller steps".

One small uncomfort in that approach is that it often is not very
obvious by reading "log -p master.." alone how well the end result
fits together.  Each individual step may make sense, or at least it
may not make it any worse than the original, but until you apply the
whole series and read "diff master..." in a sitting, it is somewhat
hard to tell where you are going.  But this is not "risk" or "bad
thing"; just something that may make readers feel uncomfortable---we
are not losing anything by splitting a series into small logical
chunks.

Thanks.