Re: [PATCH 2/3] panic: improve panic_timeout calculation

From: Ingo Molnar
Date: Mon Nov 11 2013 - 08:52:31 EST



* Felipe Contreras <felipe.contreras@xxxxxxxxx> wrote:

> On Mon, Nov 11, 2013 at 7:28 AM, Ingo Molnar <mingo@xxxxxxxxxx> wrote:
> >
> > * Felipe Contreras <felipe.contreras@xxxxxxxxx> wrote:
> >
> >> On Mon, Nov 11, 2013 at 6:49 AM, Ingo Molnar <mingo@xxxxxxxxxx> wrote:
> >> >
> >> > * Felipe Contreras <felipe.contreras@xxxxxxxxx> wrote:
> >> >
> >> >> On Mon, Nov 11, 2013 at 5:32 AM, Ingo Molnar <mingo@xxxxxxxxxx> wrote:
> >> >> >
> >> >> > * Felipe Contreras <felipe.contreras@xxxxxxxxx> wrote:
> >> >> >
> >> >> >> We want to calculate the blinks per second, and instead of making it 5
> >> >> >> (1000 / (3600 / 18)), let's make it 4, so the user can see two blinks
> >> >> >> per second.
> >> >> >
> >> >> > Please use the customary changelog style we use in the kernel:
> >> >> >
> >> >> > " Current code does (A), this has a problem when (B).
> >> >> > We can improve this doing (C), because (D)."
> >> >>
> >> >> A is explained, B is empty, C is explained, D is because it makes sense.
> >
> > So one problem with your changelog is that you describe the change but
> > don't explain a couple of things - for example why you changed '3600' to
> > '1000'.
>
> Yes, I am aware of that, and it probably should, but that has nothing
> to do with (A)(B)(C) or (D).

So you knew that your changelog was defective, but you elected to
interpret reviewer feedback narrowly and chose to be intentionally
obtuse and difficult to waste even more reviewer time?

That's rather destructive behavior, which you further demonstrate
in the rest of your reply:

> >> > NAKed-by: Ingo Molnar <mingo@xxxxxxxxxx>
> >>
> >> Suit yourself, stay with your buggy code then.
> >
> > I NAK-ed your patch because your patch has several technical problems.
>
> No, this is why you NAK-ed the patch:

I very much know why I NAK-ed your patch.

> > > A is explained, B is empty, C is explained, D is because it makes
> > > sense.
> >
> > NAKed-by: Ingo Molnar <mingo@xxxxxxxxxx>
>
> That is not a technical problem, that's an allegedly administrative one.
> I said I would fix the technical issues.

You are mistaken: kernel changelogs are part of the change, a problem with
a changelog is generally considered a technical problem as well.

> > To lift the NAK you'll need to address my review feedback
> > constructively.
>
> That's exactly what I did. Addressing feedback constructively doesn't
> mean do exactly what you say without arguing.

Your reply to my routine feedback was obtuse, argumentative and needlessly
confrontative - that's not 'constructive'.

> I will resend the patches separately since you are focusing on the
> irrelevant patches and not paying attention to the one I made clear was
> the important one, muddying it. [...]

That first patch is faulty as well.

Thanks,

Ingo
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/