| Message ID | 7072cc5e-8137-762b-53a1-c4a80d19ff08@suse.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | ./CODING_STYLE adjustments | expand |
On Fri, Jul 19, 2019 at 3:19 AM Jan Beulich <JBeulich@suse.com> wrote: > > Since the behavior of "diff -p" to use an unindented label as context > identifier often makes it harder to review patches, make explicit the > requirement for labels to be indented. > > Signed-off-by: Jan Beulich <jbeulich@suse.com> This style requirement wouldn't really work with astyle as-is. Tamas
On 19.07.2019 15:18, Tamas K Lengyel wrote: > On Fri, Jul 19, 2019 at 3:19 AM Jan Beulich <JBeulich@suse.com> wrote: >> >> Since the behavior of "diff -p" to use an unindented label as context >> identifier often makes it harder to review patches, make explicit the >> requirement for labels to be indented. >> >> Signed-off-by: Jan Beulich <jbeulich@suse.com> > > This style requirement wouldn't really work with astyle as-is. Personally I view proper "diff -p" context in patches as quite a bit more important than automatic style checking. But perhaps that's just because I do quite a lot of patch review ... Jan
On Fri, Jul 19, 2019 at 7:22 AM Jan Beulich <JBeulich@suse.com> wrote: > > On 19.07.2019 15:18, Tamas K Lengyel wrote: > > On Fri, Jul 19, 2019 at 3:19 AM Jan Beulich <JBeulich@suse.com> wrote: > >> > >> Since the behavior of "diff -p" to use an unindented label as context > >> identifier often makes it harder to review patches, make explicit the > >> requirement for labels to be indented. > >> > >> Signed-off-by: Jan Beulich <jbeulich@suse.com> > > > > This style requirement wouldn't really work with astyle as-is. > > Personally I view proper "diff -p" context in patches as quite > a bit more important than automatic style checking. But perhaps > that's just because I do quite a lot of patch review ... Which is a valid point. I don't really care which style option it really is, if it helps you, I'm fine with it. But as a maintainer who does this on a volunteer basis when I have a 5-minute window, I'm not going to spend my time looking for style issues. So in the ongoing vm_event series that's under review where you explicitly called out that the vm_event maintainers have to review and point out all style issues, that's a no-go from my side. So either we have automatic style checks or I'm just going to Ack patches with style issues. Tamas
On Fri, 19 Jul 2019, Jan Beulich wrote: > Since the behavior of "diff -p" to use an unindented label as context > identifier often makes it harder to review patches, make explicit the > requirement for labels to be indented. > > Signed-off-by: Jan Beulich <jbeulich@suse.com> Acked-by: Stefano Stabellini <sstabellini@kernel.org> > --- a/CODING_STYLE > +++ b/CODING_STYLE > @@ -31,6 +31,10 @@ void fun(void) > } > } > > +Due to the behavior of GNU diffutils "diff -p", labels should be > +indented by at least one blank. Non-case labels inside switch() bodies > +are preferred to be indented the same as the block's case labels. > + > White space > ----------- > > >
--- a/CODING_STYLE +++ b/CODING_STYLE @@ -31,6 +31,10 @@ void fun(void) } } +Due to the behavior of GNU diffutils "diff -p", labels should be +indented by at least one blank. Non-case labels inside switch() bodies +are preferred to be indented the same as the block's case labels. + White space -----------
Since the behavior of "diff -p" to use an unindented label as context identifier often makes it harder to review patches, make explicit the requirement for labels to be indented. Signed-off-by: Jan Beulich <jbeulich@suse.com>