* How about using clang-format instead checkpatch for fixing style?
@ 2020-10-09 13:02 罗勇刚(Yonggang Luo)
2020-10-09 13:25 ` Paolo Bonzini
0 siblings, 1 reply; 6+ messages in thread
From: 罗勇刚(Yonggang Luo) @ 2020-10-09 13:02 UTC (permalink / raw)
To: qemu-level, Paolo Bonzini, Peter Maydell
[-- Attachment #1: Type: text/plain, Size: 143 bytes --]
We can even using clang-format to format the whole repository.
--
此致
礼
罗勇刚
Yours
sincerely,
Yonggang Luo
[-- Attachment #2: Type: text/html, Size: 316 bytes --]
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: How about using clang-format instead checkpatch for fixing style?
2020-10-09 13:02 How about using clang-format instead checkpatch for fixing style? 罗勇刚(Yonggang Luo)
@ 2020-10-09 13:25 ` Paolo Bonzini
2020-10-09 13:30 ` 罗勇刚(Yonggang Luo)
` (2 more replies)
0 siblings, 3 replies; 6+ messages in thread
From: Paolo Bonzini @ 2020-10-09 13:25 UTC (permalink / raw)
To: luoyonggang, qemu-level, Peter Maydell
On 09/10/20 15:02, 罗勇刚(Yonggang Luo) wrote:
> We can even using clang-format to format the whole repository.
checkpatch does other checks than formatting.
Reformatting the whole repository has been mentioned many times, people
were worried of messing up the result of "git blame". But without doing
that, it's hard to get rid of checkpatch because checkpatch only looks
at the lines that are touched by the patch.
Paolo
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: How about using clang-format instead checkpatch for fixing style?
2020-10-09 13:25 ` Paolo Bonzini
@ 2020-10-09 13:30 ` 罗勇刚(Yonggang Luo)
2020-10-09 13:38 ` Daniel P. Berrangé
2020-10-09 13:48 ` 罗勇刚(Yonggang Luo)
2 siblings, 0 replies; 6+ messages in thread
From: 罗勇刚(Yonggang Luo) @ 2020-10-09 13:30 UTC (permalink / raw)
To: Paolo Bonzini; +Cc: Peter Maydell, qemu-level
[-- Attachment #1: Type: text/plain, Size: 1489 bytes --]
On Fri, Oct 9, 2020 at 9:25 PM Paolo Bonzini <pbonzini@redhat.com> wrote:
>
> On 09/10/20 15:02, 罗勇刚(Yonggang Luo) wrote:
> > We can even using clang-format to format the whole repository.
>
> checkpatch does other checks than formatting.
>
> Reformatting the whole repository has been mentioned many times, people
> were worried of messing up the result of "git blame". But without doing
> that, it's hard to get rid of checkpatch because checkpatch only looks
> at the lines that are touched by the patch.
Make sense, clang-format can also only formatting the changed lines,
checkpatch also does what? do you means about the MAINTAINER? that can be
preserved
we can use clang-format as a part of checkpatch.
The current problem with checkpatch is that is doesn't understand C well.
Sometimes will arise werid errors.
For example, what's does this means?
25/30 Checking commit 9e2a36e4c279 (plugin: Getting qemu-plugin.h can be
included in multiple source file)
ERROR: storage class should be at the beginning of the declaration
#74: FILE: include/qemu/qemu-plugin.h:432:
+#define qemu_plugin_decl_symbol(symbol_name) extern symbol_name##_t
symbol_name
total: 1 errors, 0 warnings, 88 lines checked
Patch 25/30 has style problems, please review. If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
>
> Paolo
>
--
此致
礼
罗勇刚
Yours
sincerely,
Yonggang Luo
[-- Attachment #2: Type: text/html, Size: 1720 bytes --]
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: How about using clang-format instead checkpatch for fixing style?
2020-10-09 13:25 ` Paolo Bonzini
2020-10-09 13:30 ` 罗勇刚(Yonggang Luo)
@ 2020-10-09 13:38 ` Daniel P. Berrangé
2020-10-09 13:44 ` 罗勇刚(Yonggang Luo)
2020-10-09 13:48 ` 罗勇刚(Yonggang Luo)
2 siblings, 1 reply; 6+ messages in thread
From: Daniel P. Berrangé @ 2020-10-09 13:38 UTC (permalink / raw)
To: Paolo Bonzini; +Cc: Peter Maydell, luoyonggang, qemu-level
On Fri, Oct 09, 2020 at 03:25:04PM +0200, Paolo Bonzini wrote:
> On 09/10/20 15:02, 罗勇刚(Yonggang Luo) wrote:
> > We can even using clang-format to format the whole repository.
>
> checkpatch does other checks than formatting.
>
> Reformatting the whole repository has been mentioned many times, people
> were worried of messing up the result of "git blame". But without doing
> that, it's hard to get rid of checkpatch because checkpatch only looks
> at the lines that are touched by the patch.
It is a no-win situation.
checkpatch.pl is code that makes people run away screaming in horror,
because who really wants to look at Perl code that tries to parse C
code with regexes. The fact that you can submit a patch and get
complaints about things you didn't actually change is a poor experiance,
especially for new contributors who will wonder what they did wrong.
Certain subsystem maintainers have done bulk cleanups for pieces of
code before, most notably culling tabs. So we have taken the pain
a little before. I presume we'll continue to periodically clean
code.
Aside from the git blame pain, there will also be a period of time
when cherry-picking patches back to old versions will be tediously
conflicting, potentially forcing cherr-picking of the style cleanup
patches too. If the cleanup patches are fine grained it might not
be too terrible though.
So we have pain with current state and we have pain with use of
clang-format. The difference is the current pain is long term
ongoing pain, while the clang-format pain will be an initial
hit whose impact will slowly fade over time.
Personally I think it would be worth the change in the long
term. I should really put my money where my mouth is though and
propose it for libvirt too.
Regards,
Daniel
--
|: https://berrange.com -o- https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o- https://fstop138.berrange.com :|
|: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: How about using clang-format instead checkpatch for fixing style?
2020-10-09 13:38 ` Daniel P. Berrangé
@ 2020-10-09 13:44 ` 罗勇刚(Yonggang Luo)
0 siblings, 0 replies; 6+ messages in thread
From: 罗勇刚(Yonggang Luo) @ 2020-10-09 13:44 UTC (permalink / raw)
To: Daniel P. Berrangé; +Cc: Paolo Bonzini, qemu-level, Peter Maydell
[-- Attachment #1: Type: text/plain, Size: 2539 bytes --]
On Fri, Oct 9, 2020 at 9:38 PM Daniel P. Berrangé <berrange@redhat.com>
wrote:
>
> On Fri, Oct 09, 2020 at 03:25:04PM +0200, Paolo Bonzini wrote:
> > On 09/10/20 15:02, 罗勇刚(Yonggang Luo) wrote:
> > > We can even using clang-format to format the whole repository.
> >
> > checkpatch does other checks than formatting.
> >
> > Reformatting the whole repository has been mentioned many times, people
> > were worried of messing up the result of "git blame". But without doing
> > that, it's hard to get rid of checkpatch because checkpatch only looks
> > at the lines that are touched by the patch.
>
> It is a no-win situation.
>
> checkpatch.pl is code that makes people run away screaming in horror,
> because who really wants to look at Perl code that tries to parse C
> code with regexes. The fact that you can submit a patch and get
> complaints about things you didn't actually change is a poor experiance,
> especially for new contributors who will wonder what they did wrong.
>
> Certain subsystem maintainers have done bulk cleanups for pieces of
> code before, most notably culling tabs. So we have taken the pain
> a little before. I presume we'll continue to periodically clean
> code.
>
> Aside from the git blame pain, there will also be a period of time
> when cherry-picking patches back to old versions will be tediously
> conflicting, potentially forcing cherr-picking of the style cleanup
> patches too. If the cleanup patches are fine grained it might not
> be too terrible though.
>
> So we have pain with current state and we have pain with use of
> clang-format. The difference is the current pain is long term
> ongoing pain, while the clang-format pain will be an initial
> hit whose impact will slowly fade over time.
>
> Personally I think it would be worth the change in the long
> term. I should really put my money where my mouth is though and
> propose it for libvirt too.
When the repository getting bigger and bigger, a automatically formatter
are more
and more needed,
LLVM/Chrome(Blink)/Rust and may big project are already using formatter,
python also did that, qemu are getting bigger and bigger everyday.
>
> Regards,
> Daniel
> --
> |: https://berrange.com -o-
https://www.flickr.com/photos/dberrange :|
> |: https://libvirt.org -o-
https://fstop138.berrange.com :|
> |: https://entangle-photo.org -o-
https://www.instagram.com/dberrange :|
>
--
此致
礼
罗勇刚
Yours
sincerely,
Yonggang Luo
[-- Attachment #2: Type: text/html, Size: 3290 bytes --]
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: How about using clang-format instead checkpatch for fixing style?
2020-10-09 13:25 ` Paolo Bonzini
2020-10-09 13:30 ` 罗勇刚(Yonggang Luo)
2020-10-09 13:38 ` Daniel P. Berrangé
@ 2020-10-09 13:48 ` 罗勇刚(Yonggang Luo)
2 siblings, 0 replies; 6+ messages in thread
From: 罗勇刚(Yonggang Luo) @ 2020-10-09 13:48 UTC (permalink / raw)
To: Paolo Bonzini; +Cc: Peter Maydell, qemu-level
[-- Attachment #1: Type: text/plain, Size: 735 bytes --]
On Fri, Oct 9, 2020 at 9:25 PM Paolo Bonzini <pbonzini@redhat.com> wrote:
>
> On 09/10/20 15:02, 罗勇刚(Yonggang Luo) wrote:
> > We can even using clang-format to format the whole repository.
>
> checkpatch does other checks than formatting.
>
> Reformatting the whole repository has been mentioned many times, people
> were worried of messing up the result of "git blame". But without doing
> that, it's hard to get rid of checkpatch because checkpatch only looks
> at the lines that are touched by the patch.
For people really worried about the git blame, whe can even using
clang-format
to format the whole git history:)
>
> Paolo
>
--
此致
礼
罗勇刚
Yours
sincerely,
Yonggang Luo
[-- Attachment #2: Type: text/html, Size: 936 bytes --]
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2020-10-09 13:50 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-10-09 13:02 How about using clang-format instead checkpatch for fixing style? 罗勇刚(Yonggang Luo)
2020-10-09 13:25 ` Paolo Bonzini
2020-10-09 13:30 ` 罗勇刚(Yonggang Luo)
2020-10-09 13:38 ` Daniel P. Berrangé
2020-10-09 13:44 ` 罗勇刚(Yonggang Luo)
2020-10-09 13:48 ` 罗勇刚(Yonggang Luo)
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.