Re: [PATCH v3 05/11] ref-filter: add parse_opt_merge_filter()

On Wed, Jun 17, 2015 at 1:57 PM, Matthieu Moy
[off-list ref] wrote:

Err, for-each-ref already uses it before this series, no?

So, you don't need any extra option to get for-each-ref, because it is
already there. Having these extra options is a good side effect, though.

To make sure I'm clear enough, what you're doing is

- add all options to for-each-ref
- port tag.c
- port branch.c

What I'm suggesting is to prioritize this way

- add all options required for tag.c
- port tag.c
- add all options required for branch.c
- port branch.c

I meant somewhat on those lines only.  Let me clear that out.
The steps I plan to take are:
1. Move code from for-each-ref to ref-filter.
2. Add options to ref-filter which is available in tag.c and branch.c
(--points-at, --contains, --merged)
3. Add there options to for-each-ref.
4. Add options required for functioning of tag.c alone to ref-filter
5. Port tag.c
6. Add options required for functioning of branch.c alone to ref-filter
7. Port branch.c

Now why i want to complete step 2 right after 1 is so that while
porting tag.c and branch.c
I do not want to focus on making those common options available.
Because I rather
work on getting their specific options (verbose in branch.c, -n in
tag.c and so on)
working before porting over tag.c/branch.c.

Not only the challenge, but also the way to validate your work. Think of
it as a rather comprehensive set of tests that you get for free once you
ported a command.

BTW, talking about tests, did you do some coverage analysis on git
branch and git tag? If not, I'd suggest that you do this to make sure
that the pieces of code you're rewritting using ref-filter are well
tested before being rewritten (a bit like Paul's work on shell -> C).
You don't have to actually do this before porting, but it should come
befor the port in the patch series to make sure that tests pass both the
old and new implementation.

Yes good point.

I did not do a deep coverage analysis on git tag and its tests. You are right
this would be a crucial step for porting. I had a glance over the tests. Will
look into it. As for git branch I'll do that after porting tag.c over to using
ref-filter.

-- 
Regards,
Karthik Nayak