All of lore.kernel.org
 help / color / mirror / Atom feed
From: Matthieu Moy <Matthieu.Moy@grenoble-inp.fr>
To: Karthik Nayak <karthik.188@gmail.com>
Cc: Junio C Hamano <gitster@pobox.com>,
	git@vger.kernel.org, christian.couder@gmail.com
Subject: Re: [WIP/PATCH v5 05/10] for-each-ref: introduce 'ref_array_clear()'
Date: Tue, 09 Jun 2015 08:31:15 +0200	[thread overview]
Message-ID: <vpq1thlmlzg.fsf@anie.imag.fr> (raw)
In-Reply-To: <5575F8CB.8080009@gmail.com> (Karthik Nayak's message of "Tue, 09 Jun 2015 01:49:23 +0530")

Karthik Nayak <karthik.188@gmail.com> writes:

> On 06/08/2015 10:51 PM, Matthieu Moy wrote:

>> We could introduce ref-filter.h earlier, indeed. To me, the current
>> solution is good enough, but introducing ref-filter.h early and adding
>> function definition there in the same commit as you drop the "static"
>> keyword for them would clearly be an improvement.
>
> But that would break the flow, wouldn't it? I wanted ref-filter to be
> introduced together, hence right after ref-filter.h we move code to
> ref-filter.c

That's why I find the current solution good enough: it also has
advantages. But in the current series, when you say "make functions
public", you are not actually doing so since they're not exported in a
.h file.

Conversely, PATCH 07 does two things: move code from for-each-ref.c and
introduce new declarations. Had you introduced these declarations
earlier, this patch would have been pure code movement.

In both cases, you have intermediate states that are not fully
consistant: either you have public functions in the builtin/ directory
(which sometimes happen in Git's codebase, but we try to avoid it), or
you have non-static functions that are not declared in a .h.

-- 
Matthieu Moy
http://www-verimag.imag.fr/~moy/

  reply	other threads:[~2015-06-09  6:31 UTC|newest]

Thread overview: 27+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-06-06  7:04 [WIP/PATCH v5 0/10] create ref-filter from for-each-ref Karthik Nayak
2015-06-06  7:09 ` [WIP/PATCH v5 01/10] for-each-ref: extract helper functions out of grab_single_ref() Karthik Nayak
2015-06-06  7:09   ` [WIP/PATCH v5 02/10] for-each-ref: clean up code Karthik Nayak
2015-06-08 14:37     ` Matthieu Moy
2015-06-08 15:21       ` Karthik Nayak
2015-06-08 16:23         ` Matthieu Moy
2015-06-06  7:09   ` [WIP/PATCH v5 03/10] for-each-ref: rename 'refinfo' to 'ref_array_item' Karthik Nayak
2015-06-08 14:42     ` Matthieu Moy
2015-06-08 15:19       ` Karthik Nayak
2015-06-06  7:09   ` [WIP/PATCH v5 04/10] for-each-ref: introduce new structures for better organisation Karthik Nayak
2015-06-06  7:09   ` [WIP/PATCH v5 05/10] for-each-ref: introduce 'ref_array_clear()' Karthik Nayak
2015-06-08 14:53     ` Matthieu Moy
2015-06-08 15:18       ` Karthik Nayak
2015-06-08 15:18       ` Karthik Nayak
2015-06-08 16:26         ` Matthieu Moy
2015-06-08 17:05           ` Junio C Hamano
2015-06-08 17:21             ` Matthieu Moy
2015-06-08 20:19               ` Karthik Nayak
2015-06-09  6:31                 ` Matthieu Moy [this message]
2015-06-06  7:09   ` [WIP/PATCH v5 06/10] for-each-ref: rename some functions and make them public Karthik Nayak
2015-06-06  7:09   ` [WIP/PATCH v5 07/10] ref-filter: add 'ref-filter.h' Karthik Nayak
2015-06-06  7:09   ` [WIP/PATCH v5 08/10] ref-filter: move code from 'for-each-ref' Karthik Nayak
2015-06-06  7:09   ` [WIP/PATCH v5 09/10] for-each-ref: introduce filter_refs() Karthik Nayak
2015-06-06  7:09   ` [WIP/PATCH v5 10/10] ref-filter: make 'ref_array_item' use a FLEX_ARRAY for refname Karthik Nayak
2015-06-06  9:40 ` [WIP/PATCH v5 0/10] create ref-filter from for-each-ref Christian Couder
2015-06-06 10:53   ` Karthik Nayak
2015-06-06 11:26 ` Karthik Nayak

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=vpq1thlmlzg.fsf@anie.imag.fr \
    --to=matthieu.moy@grenoble-inp.fr \
    --cc=christian.couder@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=karthik.188@gmail.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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.