Don't you think we should rather split up into smaller files before
start reorganizing things?
Pi Song
On Wed, Mar 18, 2009 at 4:41 PM, Junio C Hamano <gitster@xxxxxxxxx> wrote:
> pi song <pi.songs@xxxxxxxxx> writes:
>
>> diff --git a/blame.h b/blame.h
>> new file mode 100644
>> index 0000000..72d1e2a
>> --- /dev/null
>> +++ b/blame.h
>> @@ -0,0 +1,166 @@
>> +/*
>> + * for storing stats. it can be used
>> + * across multiple blame operations
>> + */
>> +struct blame_stat {
>> + int num_read_blob;
>> + int num_get_patch;
>> + int num_commits;
>> +};
>
> As I said in my previous message, I do not understand why this is not part
> of the super-scoreboard (now blame_info).
>
>> +#define PICKAXE_BLAME_MOVE 01
>> +#define PICKAXE_BLAME_COPY 02
>> +#define PICKAXE_BLAME_COPY_HARDER 04
>> +#define PICKAXE_BLAME_COPY_HARDEST 010
>> +
>> +#define BLAME_DEFAULT_MOVE_SCORE 20
>> +#define BLAME_DEFAULT_COPY_SCORE 40
>> +
>> +/* bits #0..7 in revision.h, #8..11 used for merge_bases() in commit.c */
>> +#define METAINFO_SHOWN (1u<<12)
>> +#define MORE_THAN_ONE_PATH (1u<<13)
>
> Do we need to expose all of these constants outside blame.c? I think the
> library caller needs access to the first four above, but I tend to think
> the latter four are purely internal implementation detail that should be
> kept in blame.c.
>
>> +/* output formatting constants */
>> +#define OUTPUT_ANNOTATE_COMPAT 001
>> +#define OUTPUT_LONG_OBJECT_NAME 002
>> +#define OUTPUT_RAW_TIMESTAMP 004
>> +#define OUTPUT_PORCELAIN 010
>> +#define OUTPUT_SHOW_NAME 020
>> +#define OUTPUT_SHOW_NUMBER 040
>> +#define OUTPUT_SHOW_SCORE 0100
>> +#define OUTPUT_NO_AUTHOR 0200
>
> I think these can be public.
>
>> +/*
>> + * One blob in a commit that is being suspected
>> + */
>> +struct origin {
>> + int refcnt;
>> + struct origin *previous;
>> + struct commit *commit;
>> + mmfile_t file;
>> + unsigned char blob_sha1[20];
>> + char path[FLEX_ARRAY];
>> +};
>
> I somehow doubt we would want to expose this level of implementation
> detail to the callers of the library. If we need to, the structure needs
> to be renamed---"origin" is way too generic a name.
>
>> +extern void assign_blame(struct blame_scoreboard *sb, int opt) ;
>
> Lose the extra SP before ";". I had to fix them in your previous patch
> and there were many.
>
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
