[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH 1/7] xz: add fall-through comments to a switch statement



Hi Jan,

On 26/11/2021 07:37, Jan Beulich wrote:
On 25.11.2021 18:13, Julien Grall wrote:
Hi,

On 25/11/2021 17:03, Jan Beulich wrote:
On 25.11.2021 17:54, Julien Grall wrote:
On 25/11/2021 16:49, Julien Grall wrote:
On 19/11/2021 10:21, Jan Beulich wrote:
From: Lasse Collin <lasse.collin@xxxxxxxxxxx>

It's good style. I was also told that GCC 7 is more strict and might
give a warning when such comments are missing.

Suggested-by: Andrei Borzenkov <arvidjaar@xxxxxxxxx>
Signed-off-by: Lasse Collin <lasse.collin@xxxxxxxxxxx>

Actually, any reason why there are some signed-off-by missing?

I often keep the author's, but drop ones which clearly got there only
because of the path a patch has taken through trees.

This might be clear for you. For me, as a reviewer, I have to do extra
work to check whether you keeped the relevant signed-off-by.

These aren't
relevant imo when pulling over the change;

They are technically part of the "chain of approval".

But the Linux chain of approval is precisely what is of no interest to
us. We need to approve the change ourselves; Linux having had it
approved is merely a data point.

I can understand this point of view. But as I wrote above, a reviewer as to do extra work to check you correctly propagated the signed-off-by (see more below).


I could as well take the
email submission as my basis, after all, where just the single S-o-b
would be there.

That's a fair point. That said, you took the commit-as-is from linus.git

How would you be able to tell?

That's easy. You wrote in your commit message:

[Linux commit: 5a244f48ecbbd03a11eb84819c5c599db81823ee]

That's indicating you used the Linux commit rather than the one on the ML. So I will tend to diff the commit and the what's different.


so I think we ought to keep them.

I disagree. And I'd like to remain consistent with what I've been doing
in the past.
I actually tried to find the original submission but failed. I didn't look for long though...

Anyway, I think it would save time for everyone (you had to manually delete signed-off-by after all) if you just copy the commit (including all the signed-off-by) message as-is.

Cheers,

--
Julien Grall



 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.