On Sat, Sep 26, 2020 at 02:48:44PM +0200, Martin Ågren wrote:
> On Fri, 25 Sep 2020 at 09:09, Jeff King <peff@xxxxxxxx> wrote:
> > +static inline int has_multi_bits(unsigned n)
> > +{
> > + return (n & (n - 1)) != 0;
> > +}
>
> There's a HAS_MULTI_BITS macro in git-compat-util.h. I like how you came
> up with the exact same name. It even makes me wonder if you are actively
> avoiding it in favor of a re-implementation, but I can't see why.
Heh. I _thought_ we had such a helper, and I even thought I remembered
the name. But when I grepped for it, I couldn't come up with it. Silly
"-i". (So I guess it's no surprise that I came up with the same
implementation, which is the only good one, and the same name, which I
did pull from the back of my mind).
I'll drop this in favor of the macro.
> > enum {
> > - SHORTLOG_GROUP_AUTHOR = 0,
> > - SHORTLOG_GROUP_COMMITTER,
> > - SHORTLOG_GROUP_TRAILER
> > - } group;
> > + SHORTLOG_GROUP_AUTHOR = (1 << 0),
> > + SHORTLOG_GROUP_COMMITTER = (1 << 1),
> > + SHORTLOG_GROUP_TRAILER = (1 << 2)
> > + } groups;
>
> Coming back to the comment on 2/8, adding a comma at the end wouldn't
> reduce patch noise here and now, but whenever someone touches this place
> the next time.
Agreed, will do.
> Of my comments in the last few mails, probably the only actionable one
> is the truncated sentence in the trailer iterator header file.
I agree none are big, but worth addressing since I was making a re-roll
anyway. Thanks.
-Peff
