* 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 a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).