Hi Martin
On 2019-06-13 19:21 UTC Martin Ågren <[email protected]> wrote:
>
> > > + const char *in_progress_advice;
> > > + const char *in_progress_error = NULL;
>
> The assigning vs not assigning is a bit inconsistent, but that's a very
> minor nit, and not why I started replying. Only noticed it just now. :-)
Let's make both NULL then.
> I agree 100% with Phillip, but I'll also note that "the control must not
> reach here" doesn't tell me anything that BUG() doesn't already. That
> is, the point of BUG() is to document that, indeed, we shouldn't get
> here and to alert if we do anyway.
Agreed.
> An obvious alternative would be
>
> BUG("action is neither revert nor pick");
>
> but that doesn't say much more than the code already says quite clearly,
> plus it risks getting outdated. I'd probably settle on something like
>
> BUG("unexpected action in create_seq_dir");
Yeah, now that I know more about what BUG is truly intended for, I
think you are right here. We should change it.
> which should give us a good clue even if all we have is this message (so
> no file, no line number), but I am sure there are other good choices
> here. :-)
>
> Thanks Rohit for your work on this. I'm impressed by how you've polished
> this series.
Thank you very much Martin. Appreciations like these help us developers
keep going and working even harder.
Thanks
Rohit
