> 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.
Thanks for clarifying.
> >> #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.
For reference, the discussion is here [1]. Szeder Gábor has a dissenting
argument in [2] that makes sense to me (gdb on my desktop also shows an
enum used as a bitset as "(A | B)"). But in any case, perhaps we should
decide if it's fine to use an enum here before Wambui Karuga continues
development.
[1] https://public-inbox.org/git/xmqqsgobg0rv....@gitster-ct.c.googlers.com/
[2] https://public-inbox.org/git/20191007171249.gb11...@szeder.dev/
> > 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).
Good point.
