Jonathan Tan <jonathanta...@google.com> writes:
>> - if ((opt & PICKAXE_BLAME_COPY_HARDEST)
>> - || ((opt & PICKAXE_BLAME_COPY_HARDER)
>> + if ((opt & BLAME_COPY_HARDEST)
>> + || ((opt & BLAME_COPY_HARDER)
>
> Any reason why the names are renamed to omit "PICKAXE_"? In particular,
> these names are still global, so it is good to retain the extra context.
Absolutely. Removing them is wrong, I would have to say.
>> #define BLAME_DEFAULT_MOVE_SCORE 20
>> #define BLAME_DEFAULT_COPY_SCORE 40
>>
>> +enum pickaxe_blame_action {
>> + BLAME_MOVE = 01,
>> + BLAME_COPY,
>> + BLAME_COPY_HARDER = 04,
>> + BLAME_COPY_HARDEST = 010,
>> +};
We had a bit of discussion recently about using (or rather, not
abusing) enum for set of bits on a different topic.
> Also, I have a slight preference for putting "= 02" on the BLAME_COPY
> line but that is not necessary.
That is absolutely necessary; it is not like "we do not care what
exact value _COPY gets; it can be any value as long as it is _MOVE
plus 1", as these are used in set of bits (and again, I do not think
it is such a brilliant idea to use enum for such a purpose).
Thanks.
