Re: [PATCH v2 01/18] cat-file: split expand_atom into 2 functions

["Оля Тележная" <olyatelezhnaya@gmail.com>]

2018-01-18 2:04 GMT+03:00 Christian Couder <christian.couder@gmail.com>:
> On Wed, Jan 17, 2018 at 10:49 PM, Jeff King <peff@peff.net> wrote:
>> On Tue, Jan 16, 2018 at 10:22:23AM +0300, Оля Тележная wrote:
>>
>>> >> In other words, I think the endgame is that expand_atom() isn't there at
>>> >> all, and we're calling the equivalent of format_ref_item() for each
>>> >> object (except that in a unified formatting world, it probably doesn't
>>> >> have the word "ref" in it, since that's just one of the items a caller
>>> >> might pass in).
>>>
>>> Agree! I want to merge current edits, then create format.h file and
>>> make some renames, then finish migrating process to new format.h and
>>> support all new meaningful tags.
>>
>> I think we have a little bit of chicken and egg there, though. I'm
>> having trouble reviewing the current work, because it's hard to evaluate
>> whether it's doing the right thing without seeing the end state.
>
> Yeah, to me it feels like you are at a middle point and there are many
> ways to go forward.

OK. Maybe I misunderstood you and Jeff in our call, I thought that was
your idea to make a merge now, sorry. I will continue my work here.

>
> As I wrote in another email though, I think it might be a good time to
> consolidate new functionality by adding tests (and perhaps
> documentation at the same time) for each new atom that is added to
> ref-filter or cat-file. It will help you refactor the code and your
> patch series later without breaking the new functionality.
>
>> So what
>> I was suggesting in my earlier mails was that we actually _not_ try to
>> merge this series, but use its components and ideas to build a new
>> series that does things in a bit different order.
>
> Yeah, I think you will have to do that, but the tests that you can add
> now for the new features will help you when you will build the new
> series.
>
> And hopefully it will not be too much work to create this new series
> as you will perhaps be able to just use the interactive rebase to
> build it.
>
> I also don't think it's a big problem if the current patch series gets
> quite long before you start creating a new series.