Re: [PATCH/WIP v2 04/19] am: implement patch queue mechanism

From: Paul Tan <pyokagan@gmail.com>
Date: Jun 15, 2015, 10:46 UTC
Message-ID: <CACRoPnS3QwoWoebDpz_aBeoGx91pdP68f63Qp5KxLk9BCvc7DQ@mail.gmail.com>
In-Reply-To: <CAGZ79kbLneLygPiDtL+nqwsxdvZEhT95vj0z9MbN0QrqvAmebA@mail.gmail.com>

On Fri, Jun 12, 2015 at 1:39 AM, Stefan Beller <sbeller@google.com> wrote:

Show 24 quoted lines

> On Thu, Jun 11, 2015 at 3:21 AM, Paul Tan <pyokagan@gmail.com> wrote:
>> Notes:
>>     v2
>>
>>     * Declare struct am_state as static
>>
>>  builtin/am.c | 164 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
>>  1 file changed, 164 insertions(+)
>>
>> diff --git a/builtin/am.c b/builtin/am.c
>> index 0ccbe33..f061d21 100644
>> --- a/builtin/am.c
>> +++ b/builtin/am.c
>> @@ -6,6 +6,154 @@
>>  #include "cache.h"
>>  #include "builtin.h"
>>  #include "exec_cmd.h"
>> +#include "parse-options.h"
>> +#include "dir.h"
>> +
>> +struct am_state {
>
> Did you mean to declare all the functions below to be static or the
> struct as well?

Well, everything in this file, with the exception of the cmd_am() entry point, should have static linkage.

The changelog comment was referring to [1], but I should have made it clearer. Sorry if it was confusing.

[1] http://thread.gmane.org/gmane.comp.version-control.git/270048/focus=270205

> Reading further, you declared it static below. I thought maybe it'd be
> useful to have definition
> and declaration up here, but having all declarations further below may
> be even better.

Right, I aimed to have a strict separation between "git-am: the functionality" and "git-am: the command-line interface", where the latter depends on the former, and not the other way round (or have circular dependencies). The former perhaps could even be moved into libgit.a in the future.

Thanks, Paul