Jeff King <peff@xxxxxxxx> writes:
> It's just that I found myself writing up notes like "this should be
> merged", and it occurred to me that I could communicate the same things
> by sending you a proposed history. So I'm curious if you find dissecting
> it via "git log" more or less convenient than a list of notes. :)
Yes, I think this is a very readable form of communication.
What's especially nice in what you did is that comparing outputs of
these two commands
$ git log --no-merges --oneline master..peff/for-junio/master
$ git log --no-merges --oneline ^pu master..peff/for-junio/master
clearly shows that you reused what I already had in 'pu' and the
ones missing from 'pu' are the ones I need to check and possibly
may want to sign-off myself.
I also need to note that while this is very handy format for the
recipient, hence a very good way to do "pass the pumpkin" between
maintainers, it is a less "open" way of communication than laying
out everything on the list (cf. http://youtu.be/L8OOzaqS37s), but
it is the most appropriate method in this case, I would think.
> It looks like Johannes prefers replacements for some of the fixes from
> yesterday.
As to that change, I agree with you that I do not care too deeply
about the shape of an interim step as long as the finished product
uses the better one between the alternatives, but at the same time,
because we are including it as a hot-fix in a released product, even
though it is an interim step in the bigger picture, I want it to be
using the better one, too. So I am leaning towards taking what you
queued for me for the reasons you stated in the other thread [*1*].
That would probably mean Dscho needs a bit of rebase in the
remaining parts of his series that adds new elements to the enum,
but I am hoping that he can afford that time now in the feature
freeze period. If we take the _INVALID thing instead now, the
remaining series from Dscho that we would review and queue for the
next cycle would need to begin with a "fixup" patch that stops doing
the _INVALID thing and instead using the "compare after casting to
size_t", so it is just the matter of doing the adjustment before or
after the remainder of the series are posted on the list for the
next release, and the smaller number of "oops that was not nice,
let's fix it" patches in the published history, the better the long
term health of the project, I would think.
The other fixes in your collection looked all sensible. Thanks.
[Footnote]
*1* The convention of keeping _INVALID at the end can be arguably
made safe from future programmer screw-ups, as long as it is
guarded by a good comment nearby. But the solution to cast
would avoid having to have that potential brittleness in the
first place. I find the other merit of taking care of negative
enum automatically quite attractive, too.