[U-Boot] Policy for checkpatch usage?
Hi,
currently our CodingStyle[1] has this to say about checkpatch.pl:
Make sure to run the checkpatch.pl script from the Linux source tree to check your patches. Note that this should be done before posting on the mailing list!
Now this is not very clear as to what should be done with regard to the results of such a checkpatch run. I think we all agree that warnings tailored to the linux kernel like this can be ignored:
WARNING: consider using kstrto* in preference to simple_strtol #215: FILE: net/tftp.c:619: + TftpRemotePort = simple_strtol(ep, NULL, 10);
From this it follows, that we _cannot_ require 0 warnings from
checkpatch. Maybe we can require 0 _errors_?
Moreover, it's only recently that I saw checkpatch warning about lines that are not even changed in a patch but are only in the context, as for exmaple here:
WARNING: unnecessary whitespace before a quoted newline #293: FILE: net/net.c:1604: debug("Got ICMP ECHO REQUEST, return %d bytes \n",
Now when this is "fixed", the new patch will then contain a cosmetic change only intermixed with the "real" changes. Personally I think this is problematic and contrary to the intention of checking a _patch_, but it's the way it is.
I think we all agree that we should make reviews as easy as possible. In order for that, we need to separate cosmetic from functional changes. Otherwise in large patches it is very difficult to find the meat of a patch. Only recently this exact problem made Andy Fleming resplit a large commit[2] into functional and cosmetic[3] changes (ironically the cosmetic changes were added as a followup to my request of fixing checkpatch problems). As much as this is appreciated, it is clear that it takes some effort to do. Ideally we should prevent such work right from the start.
So what exact wording should we use in our CodingStyle to prevent such problems, or should we even start our own version of checkpatch tailored to our project?
As a base for discussion, what about this:
Use common sense in interpreting the results of checkpatch. Warnings that clearly only make sense in the Linux kernel can be ignored. Also warnings produced for _context lines_ rather than actual changes can also be ignored.
Thanks Detlev
[1] http://www.denx.de/wiki/view/U-Boot/CodingStyle [2] http://article.gmane.org/gmane.comp.boot-loaders.u-boot/97068 [3] http://article.gmane.org/gmane.comp.boot-loaders.u-boot/97129
On Wednesday, April 20, 2011, Detlev Zundel dzu@denx.de wrote:
Hi,
As a base for discussion, what about this:
Use common sense in interpreting the results of checkpatch. Warnings that clearly only make sense in the Linux kernel can be ignored. Also warnings produced for _context lines_ rather than actual changes can also be ignored.
One man's common sense is another's idiocy
I vote for a zero warnings, zero errors U-Boot specific checkpatch
Regards,
Graeme
On Wednesday, April 20, 2011, Graeme Russ graeme.russ@gmail.com wrote:
On Wednesday, April 20, 2011, Detlev Zundel dzu@denx.de wrote:
Hi,
As a base for discussion, what about this:
Use common sense in interpreting the results of checkpatch. Warnings that clearly only make sense in the Linux kernel can be ignored. Also warnings produced for _context lines_ rather than actual changes can also be ignored.
One man's common sense is another's idiocy
I vote for a zero warnings, zero errors U-Boot specific checkpatch
I also think that all patches should be submitted with a checkpatch summary with an explaination for any errors or warnings - this will at least save a little effort for the maintainers and reduce the number of patches bounced only to have the checkpatch problems argued away by the author anyway
Regards,
Graeme
Hi Graeme,
On Wednesday, April 20, 2011, Graeme Russ graeme.russ@gmail.com wrote:
On Wednesday, April 20, 2011, Detlev Zundel dzu@denx.de wrote:
Hi,
As a base for discussion, what about this:
Use common sense in interpreting the results of checkpatch. Warnings that clearly only make sense in the Linux kernel can be ignored. Also warnings produced for _context lines_ rather than actual changes can also be ignored.
One man's common sense is another's idiocy
I vote for a zero warnings, zero errors U-Boot specific checkpatch
I also think that all patches should be submitted with a checkpatch summary with an explaination for any errors or warnings - this will at least save a little effort for the maintainers and reduce the number of patches bounced only to have the checkpatch problems argued away by the author anyway
When we accept 0 errors and 0 warnings only, then we will always see the same text :)
As long as we are not there, I do agree but then we should come up with a recipe on how to automate this. I looked into git format-patch but it does not seem to have such an option. Does anyone have a clever one-liner for this?
Cheers Detlev
Hi Graeme,
On Wednesday, April 20, 2011, Detlev Zundel dzu@denx.de wrote:
Hi,
As a base for discussion, what about this:
Use common sense in interpreting the results of checkpatch. Warnings that clearly only make sense in the Linux kernel can be ignored. Also warnings produced for _context lines_ rather than actual changes can also be ignored.
One man's common sense is another's idiocy
I vote for a zero warnings, zero errors U-Boot specific checkpatch
Forking checkpatch means that someone has to invest work to maintain it. Judging from the linux repo there are quite some changes in every iteration of the kernel, so this will be work for us also:
[dzu@pollux linux (master)]$ git rev-list v2.6.35..v2.6.36 scripts/checkpatch.pl | wc -l 7 [dzu@pollux linux (master)]$ git rev-list v2.6.36..v2.6.37 scripts/checkpatch.pl | wc -l 19 [dzu@pollux linux (master)]$ git rev-list v2.6.37..v2.6.38 scripts/checkpatch.pl | wc -l 4
Do we want to invest this work? Do you volunteer? ;)
Can't we disable individual messages not relevant for us? Like "-Wno-kstrto" or such things?
Cheers Detlev
On Wed, 20 Apr 2011 20:15:40 +1000 Graeme Russ graeme.russ@gmail.com wrote:
On Wednesday, April 20, 2011, Detlev Zundel dzu@denx.de wrote:
Hi,
As a base for discussion, what about this:
Use common sense in interpreting the results of checkpatch. Warnings that clearly only make sense in the Linux kernel can be ignored. Also warnings produced for _context lines_ rather than actual changes can also be ignored.
One man's common sense is another's idiocy
I vote for a zero warnings, zero errors U-Boot specific checkpatch
I vote for "checkpatch is a tool that can help you find some style problems, but is imperfect, and the things it complains about are of varying importance". If you insist on zero warnings, what's the difference between a warning and an error? And will there then be a U-Boot-specific coding style document to match? Will anyone that wants to submit a patch that checkpatch erroneously complains about have to first submit a patch for checkpatch (first learning Perl if need be)?
There's a lot more "common sense" that needs to be applied when writing software than where to stick what kind and amount of whitespace. Guidelines are good -- zero-tolerance obedience to a script, not so much.
-Scott
On Thu, Apr 21, 2011 at 2:51 AM, Scott Wood scottwood@freescale.com wrote:
On Wed, 20 Apr 2011 20:15:40 +1000 Graeme Russ graeme.russ@gmail.com wrote:
On Wednesday, April 20, 2011, Detlev Zundel dzu@denx.de wrote:
Hi,
As a base for discussion, what about this:
Use common sense in interpreting the results of checkpatch. Warnings that clearly only make sense in the Linux kernel can be ignored. Also warnings produced for _context lines_ rather than actual changes can also be ignored.
One man's common sense is another's idiocy
I vote for a zero warnings, zero errors U-Boot specific checkpatch
I vote for "checkpatch is a tool that can help you find some style problems, but is imperfect, and the things it complains about are of varying importance". If you insist on zero warnings, what's the difference between a warning and an error? And will there then be a U-Boot-specific coding style document to match? Will anyone that wants to submit a patch that checkpatch erroneously complains about have to first submit a patch for checkpatch (first learning Perl if need be)?
There's a lot more "common sense" that needs to be applied when writing software than where to stick what kind and amount of whitespace. Guidelines are good -- zero-tolerance obedience to a script, not so much.
Point taken.
What about my other suggestion - A checkpatch summary with an expalation for any warnings or errors? See for example my heads-up for checkpatch warnings - http://lists.denx.de/pipermail/u-boot/2011-April/090144.html
Regards,
Graeme
Dear Graeme Russ,
In message BANLkTimdJkETWEOrJFuxq4hN8gG+DvY=-A@mail.gmail.com you wrote:
What about my other suggestion - A checkpatch summary with an expalation for any warnings or errors? See for example my heads-up for checkpatch warnings - http://lists.denx.de/pipermail/u-boot/2011-April/090144.html
For issues like this one should do what the last lines of the checkpatch output say: "If any of these errors are false positives report them to the maintainer, see CHECKPATCH in MAINTAINERS."
Best regards,
Wolfgang Denk
Hi Wolfgang,
Dear Graeme Russ,
In message BANLkTimdJkETWEOrJFuxq4hN8gG+DvY=-A@mail.gmail.com you wrote:
What about my other suggestion - A checkpatch summary with an expalation for any warnings or errors? See for example my heads-up for checkpatch warnings - http://lists.denx.de/pipermail/u-boot/2011-April/090144.html
For issues like this one should do what the last lines of the checkpatch output say: "If any of these errors are false positives report them to the maintainer, see CHECKPATCH in MAINTAINERS."
But as I showed, we already know that we do have false positives in U-Boot as we miss some Linux constructs. We should refrain from taking this as "false positives" to the checkpatch maintainers. Either we can suppress them somehow or maybe we should list them?
Cheers Detlev
Hi Scott,
I vote for "checkpatch is a tool that can help you find some style problems, but is imperfect, and the things it complains about are of varying importance". If you insist on zero warnings, what's the difference between a warning and an error? And will there then be a U-Boot-specific coding style document to match? Will anyone that wants to submit a patch that checkpatch erroneously complains about have to first submit a patch for checkpatch (first learning Perl if need be)?
So you would agree to this text:
Checkpatch is a tool that can help you find some style problems, but is imperfect, and the things it complains about are of varying importance. So use common sense in interpreting the results. Warnings that clearly only make sense in the Linux kernel can be ignored.
What about the problem with checkpatch errors in current code, i.e. the origin of this sentence:
Also warnings produced for context lines (i.e. existing code) rather than actual changes can also be ignored.
Do we want this?
There's a lot more "common sense" that needs to be applied when writing software than where to stick what kind and amount of whitespace. Guidelines are good -- zero-tolerance obedience to a script, not so much.
I agree 100%, but I also understand that people want to see some guideline that they can refer to. So let's see how good a compromise we can find.
Thanks Detlev
On Thu, Apr 21, 2011 at 04:29:17PM +0200, Detlev Zundel wrote:
What about the problem with checkpatch errors in current code, i.e. the origin of this sentence:
Also warnings produced for context lines (i.e. existing code) rather than actual changes can also be ignored.
How about replacing it with this:
If you encounter warnings for existing code, not modified by your patch, consider submitting a separate, cosmetic-only patch -- clearly described as such -- that *precedes* your substantive patch.
At 10:49 21.04.2011 -0400, Eric Cooper wrote:
On Thu, Apr 21, 2011 at 04:29:17PM +0200, Detlev Zundel wrote:
What about the problem with checkpatch errors in current code, i.e. the origin of this sentence:
Also warnings produced for context lines (i.e. existing code) rather than actual changes can also be ignored.
How about replacing it with this:
If you encounter warnings for existing code, not modified by your patch, consider submitting a separate, cosmetic-only patch -- clearly described as such -- that *precedes* your substantive patch.
Is that even possible? The cosmetic patch itself will be surrounded by context lines which may fire up a warning. So these lines need to be changed as well to satisy checkpatch. But this new patch will again include several context lines... until you have to fix up the whole file. Or did I misunderstand?
bye Fabi
On Thu, Apr 21, 2011 at 04:56:36PM +0200, Fabian Cenedese wrote:
Is that even possible? The cosmetic patch itself will be surrounded by context lines which may fire up a warning. So these lines need to be changed as well to satisy checkpatch. But this new patch will again include several context lines... until you have to fix up the whole file. Or did I misunderstand?
What's wrong with (cosmetically) fixing all the files that a patch touches? That way the project gets incremental cleanup of the code base as it evolves.
(It would be easy to automate a check for whitespace-only patches to ease the job of the custodians. Line-breaking and other style changes might still require eyeballing.)
Hi Eric,
On Thu, Apr 21, 2011 at 04:56:36PM +0200, Fabian Cenedese wrote:
Is that even possible? The cosmetic patch itself will be surrounded by context lines which may fire up a warning. So these lines need to be changed as well to satisy checkpatch. But this new patch will again include several context lines... until you have to fix up the whole file. Or did I misunderstand?
What's wrong with (cosmetically) fixing all the files that a patch touches? That way the project gets incremental cleanup of the code base as it evolves.
Of course such a cleanup would be nice indeed, but I assume that many people do not want to go this extra mile - alas I do not want to require it.
(It would be easy to automate a check for whitespace-only patches to ease the job of the custodians. Line-breaking and other style changes might still require eyeballing.)
Like this?
Cheers Detlev
Hi Fabi,
At 10:49 21.04.2011 -0400, Eric Cooper wrote:
On Thu, Apr 21, 2011 at 04:29:17PM +0200, Detlev Zundel wrote:
What about the problem with checkpatch errors in current code, i.e. the origin of this sentence:
Also warnings produced for context lines (i.e. existing code) rather than actual changes can also be ignored.
How about replacing it with this:
If you encounter warnings for existing code, not modified by your patch, consider submitting a separate, cosmetic-only patch -- clearly described as such -- that *precedes* your substantive patch.
Is that even possible? The cosmetic patch itself will be surrounded by context lines which may fire up a warning. So these lines need to be changed as well to satisy checkpatch. But this new patch will again include several context lines... until you have to fix up the whole file. Or did I misunderstand?
It may become an iterative process. Fortunately our source files are finite, so this process has a fixpoint ;)
Cheers Detlev
Hi Eric,
On Thu, Apr 21, 2011 at 04:29:17PM +0200, Detlev Zundel wrote:
What about the problem with checkpatch errors in current code, i.e. the origin of this sentence:
Also warnings produced for context lines (i.e. existing code) rather than actual changes can also be ignored.
How about replacing it with this:
Thanks for the input. Current base for discussion:
Checkpatch is a tool that can help you find some style problems, but is imperfect, and the things it complains about are of varying importance. So use common sense in interpreting the results.
Warnings that clearly only make sense in the Linux kernel can be ignored. This includes "Use #include <linux/$file> instead of <asm/$file>" for example.
If you encounter warnings for existing code, not modified by your patch, consider submitting a separate, cosmetic-only patch -- clearly described as such -- that *precedes* your substantive patch.
Anyone unhappy with that? Will this help newcomers?
Happy easter everyone, and remember around easter some people like to hide eggs, _not_ bugs ;)
Detlev
Am Donnerstag, den 21.04.2011, 17:46 +0200 schrieb Detlev Zundel:
Thanks for the input. Current base for discussion:
Checkpatch is a tool that can help you find some style problems, but is imperfect, and the things it complains about are of varying importance. So use common sense in interpreting the results.
Warnings that clearly only make sense in the Linux kernel can be ignored. This includes "Use #include <linux/$file> instead of <asm/$file>" for example.
If you encounter warnings for existing code, not modified by your patch, consider submitting a separate, cosmetic-only patch -- clearly described as such -- that *precedes* your substantive patch.
For minor modifications (e.g. changed arguments of a function call), adhere to the present codingstyle of the module. Relating checkpatch warnings can be ignored in this case. A respective note in the commit or cover letter why they are ignored is desired.
Anyone unhappy with that? Will this help newcomers?
Sounds sensible to me. The extra paragraph above is to circumvent intermixing codingstyle inside a file. For example in common/main.c, there are "function (arg, arg)" all around. Not checkpatch compliant (and ugly, IMHO). Now when I modify one of this calls, e.g add an argument, I'd have to change this. Which leads to a mixture of styles in the file, which is even worse than than adhering to the "broken" style.
In general, I'd suggest a - maybe informal - policy that while codingstyle is important, there are more relevant aspects like elegant code, sensible interfaces, proper use of static and const and so on. I'm sure any developer is willing to debate about these issues, but most volunteers (esp. company or freelance coders) will resign after the 3rd round of whitespace ping-pong...
Not sure how to phrase that without contradicting the basic idea, maybe with an additional paragraph like: "New modules have to be as checkpatch compliant as possible. Violations have to be justified in the commit or cover letter."
Hi Andreas,
Am Donnerstag, den 21.04.2011, 17:46 +0200 schrieb Detlev Zundel:
Thanks for the input. Current base for discussion:
Checkpatch is a tool that can help you find some style problems, but is imperfect, and the things it complains about are of varying importance. So use common sense in interpreting the results.
Warnings that clearly only make sense in the Linux kernel can be ignored. This includes "Use #include <linux/$file> instead of <asm/$file>" for example.
If you encounter warnings for existing code, not modified by your patch, consider submitting a separate, cosmetic-only patch -- clearly described as such -- that *precedes* your substantive patch.
For minor modifications (e.g. changed arguments of a function call), adhere to the present codingstyle of the module. Relating checkpatch warnings can be ignored in this case. A respective note in the commit or cover letter why they are ignored is desired.
Anyone unhappy with that? Will this help newcomers?
Sounds sensible to me. The extra paragraph above is to circumvent intermixing codingstyle inside a file. For example in common/main.c, there are "function (arg, arg)" all around. Not checkpatch compliant (and ugly, IMHO). Now when I modify one of this calls, e.g add an argument, I'd have to change this. Which leads to a mixture of styles in the file, which is even worse than than adhering to the "broken" style.
Thanks, I've updated the wiki page accordingly[1].
In general, I'd suggest a - maybe informal - policy that while codingstyle is important, there are more relevant aspects like elegant code, sensible interfaces, proper use of static and const and so on. I'm sure any developer is willing to debate about these issues, but most volunteers (esp. company or freelance coders) will resign after the 3rd round of whitespace ping-pong...
I fully agree, but on the other hand if we do have a Codingstyle document _and_ a tool to check for it, there should be no more than one round and even this one round should only be neccessary if the poster didn't previously read the Codingstyle document ;)
Not sure how to phrase that without contradicting the basic idea, maybe with an additional paragraph like: "New modules have to be as checkpatch compliant as possible. Violations have to be justified in the commit or cover letter."
Let's skip this. My experience is that people want clear and sharp-cut policies so that they can decide for themselves _before_ posting whether they adhere to them. If we cannot achieve such a strictness in our forumlation, I expect such clauses to generate more discussion than they solve.
Thanks everyone for the input! Detlev
[1] http://www.denx.de/wiki/U-Boot/Patches
On Thu, 21 Apr 2011 16:29:17 +0200 Detlev Zundel dzu@denx.de wrote:
Hi Scott,
I vote for "checkpatch is a tool that can help you find some style problems, but is imperfect, and the things it complains about are of varying importance". If you insist on zero warnings, what's the difference between a warning and an error? And will there then be a U-Boot-specific coding style document to match? Will anyone that wants to submit a patch that checkpatch erroneously complains about have to first submit a patch for checkpatch (first learning Perl if need be)?
So you would agree to this text:
Checkpatch is a tool that can help you find some style problems, but is imperfect, and the things it complains about are of varying importance. So use common sense in interpreting the results. Warnings that clearly only make sense in the Linux kernel can be ignored.
Yes.
That said, if someone wants to maintain a U-Boot version, that'd be great.
What about the problem with checkpatch errors in current code, i.e. the origin of this sentence:
Also warnings produced for context lines (i.e. existing code) rather than actual changes can also be ignored.
Do we want this?
That sounds like a bug in checkpatch, which should be reported.
But there's a similar issue of new code that is added in compliance with the existing style in the file, which is slightly different from what checkpatch wants. I'd usually be inclined to stick with what's already in the file, as long as it's not too ugly, rather than introduce inconsistency or reformat a large chunk of code to accommodate a small addition.
-Scott
On 22/04/11 02:10, Scott Wood wrote:
On Thu, 21 Apr 2011 16:29:17 +0200 Detlev Zundel dzu@denx.de wrote:
Hi Scott,
I vote for "checkpatch is a tool that can help you find some style problems, but is imperfect, and the things it complains about are of varying importance". If you insist on zero warnings, what's the difference between a warning and an error? And will there then be a U-Boot-specific coding style document to match? Will anyone that wants to submit a patch that checkpatch erroneously complains about have to first submit a patch for checkpatch (first learning Perl if need be)?
So you would agree to this text:
Checkpatch is a tool that can help you find some style problems, but is imperfect, and the things it complains about are of varying importance. So use common sense in interpreting the results. Warnings that clearly only make sense in the Linux kernel can be ignored.
Yes.
That said, if someone wants to maintain a U-Boot version, that'd be great.
So, if someone maintains a U-Boot fork of checkpatch, keeps it up-to-date with the Linux version, and pushes patches back up to Linux (to keep them is sync as much as practicable possible) would we agree that that would be the most favoured solution?
I'm looking at checkpatch now (and its change history) - If I think I can take it on, I will send out a call for U-Boot specific checkpatch features
Regards,
Graeme
Le 22/04/2011 02:43, Graeme Russ a écrit :
So, if someone maintains a U-Boot fork of checkpatch, keeps it up-to-date with the Linux version, and pushes patches back up to Linux (to keep them is sync as much as practicable possible) would we agree that that would be the most favoured solution?
I don't know about 'the most favoured', but I would agree that it would be a good way to implement a "zero error, zero warning" policy that actually makes sense, because we'll be the ones who decide what causes an error or warning and what does not. We could even serenely make it "absolutely zero error, ideally zero warning unless justified" if we can control which checks are warnings and which checks are errors.
I'm looking at checkpatch now (and its change history) - If I think I can take it on, I will send out a call for U-Boot specific checkpatch features
Wish you luck -- as I said, I did try once to have a fairly simple change put in the Linux checkpatch (make maximum line length a command line option), and I got zero answer, both public or private. As checkpatch compliance could be attained without this change, I eventually gave up, but a reactive 'u-checkpatch.pl' maintainer surely will attract my interest -- FWIW. :)
As for 'U-Boot specific features', I would advise to rather consider 'non-Linux-specific features'. We're having issues with the current checkpatch because it is Linux-centric (it either tests for actual Linux source-code related features or enforces 'Linux cultural' choices); replacing these Linux-specific checks with U-Boot specific checks would make the Linux and U-Boot checkpatches diverge.
So my personal Xmas wishlist #1 is to be able to choose the set of checks that will be performed and which ones will be errors vs warnings. Could be a command line option ('--linux' to apply the set of checks for a Linux patch and '--u-boot' for an U-Boot patch) or a configuration file, for instance.
Regards,
Graeme
Amicalement,
On 22/04/11 16:18, Albert ARIBAUD wrote:
Le 22/04/2011 02:43, Graeme Russ a écrit :
So, if someone maintains a U-Boot fork of checkpatch, keeps it up-to-date with the Linux version, and pushes patches back up to Linux (to keep them is sync as much as practicable possible) would we agree that that would be the most favoured solution?
I don't know about 'the most favoured', but I would agree that it would be a good way to implement a "zero error, zero warning" policy that actually makes sense, because we'll be the ones who decide what causes an error or warning and what does not. We could even serenely make it "absolutely zero error, ideally zero warning unless justified" if we can control which checks are warnings and which checks are errors.
Checks which we do not have control over using the Linux checkpatch
I'm looking at checkpatch now (and its change history) - If I think I can take it on, I will send out a call for U-Boot specific checkpatch features
Wish you luck -- as I said, I did try once to have a fairly simple change put in the Linux checkpatch (make maximum line length a command line option), and I got zero answer, both public or private. As checkpatch compliance could be attained without this change, I eventually gave up, but a reactive 'u-checkpatch.pl' maintainer surely will attract my interest -- FWIW. :)
Well, I don't know perl (yet) but the code looks very neat and nicely laid out and all the checks well documented. Recent activity has not been that extreme so I don't think keeping a U-Boot version in sync with Linux will be that hard
As for 'U-Boot specific features', I would advise to rather consider 'non-Linux-specific features'. We're having issues with the current checkpatch because it is Linux-centric (it either tests for actual Linux source-code related features or enforces 'Linux cultural' choices); replacing these Linux-specific checks with U-Boot specific checks would make the Linux and U-Boot checkpatches diverge.
I don't think the Linux guys are too concerned about white-space cleanups in patches which include functional changes, but we are
So my personal Xmas wishlist #1 is to be able to choose the set of checks that will be performed and which ones will be errors vs warnings. Could be a command line option ('--linux' to apply the set of checks for a Linux patch and '--u-boot' for an U-Boot patch) or a configuration file, for instance.
I think wrapping our requirements around a command line option is a good idea anyway, even if the Linux guys do not accept our changes to checkpatch. I want to make it so a single option makes any U-Boot fork of checkpatch behave exactly like the Linux version. That would mean combined U-Boot/Linux developers only need to worry about a single script
Regards,
Graeme
Dear Graeme Russ,
In message 4DB0CF2F.2020701@gmail.com you wrote:
That said, if someone wants to maintain a U-Boot version, that'd be great.
So, if someone maintains a U-Boot fork of checkpatch, keeps it up-to-date with the Linux version, and pushes patches back up to Linux (to keep them is sync as much as practicable possible) would we agree that that would be the most favoured solution?
I'm looking at checkpatch now (and its change history) - If I think I can take it on, I will send out a call for U-Boot specific checkpatch features
I think it wouldbe even better if we could push our changes back into the "mainline" version of checkpatch, so that the U-Boot specific behaviour can beenabled by a command line option (checkpatch --uboot ?).
Forking is not so preferrable here, I think.
Best regards,
Wolfgang Denk
On 22/04/11 18:54, Wolfgang Denk wrote:
Dear Graeme Russ,
In message 4DB0CF2F.2020701@gmail.com you wrote:
That said, if someone wants to maintain a U-Boot version, that'd be great.
So, if someone maintains a U-Boot fork of checkpatch, keeps it up-to-date with the Linux version, and pushes patches back up to Linux (to keep them is sync as much as practicable possible) would we agree that that would be the most favoured solution?
I'm looking at checkpatch now (and its change history) - If I think I can take it on, I will send out a call for U-Boot specific checkpatch features
I think it wouldbe even better if we could push our changes back into the "mainline" version of checkpatch, so that the U-Boot specific behaviour can beenabled by a command line option (checkpatch --uboot ?).
Forking is not so preferrable here, I think.
I agree, but if the Linux guys won't accept patches for U-Boot specific semantics, what choice do we have?
Regards,
Graeme
Dear Graeme Russ,
In message 4DB15DD7.8050404@gmail.com you wrote:
I think it wouldbe even better if we could push our changes back into the "mainline" version of checkpatch, so that the U-Boot specific behaviour can beenabled by a command line option (checkpatch --uboot ?).
Forking is not so preferrable here, I think.
I agree, but if the Linux guys won't accept patches for U-Boot specific semantics, what choice do we have?
Did you ask him, and he refused, or is this just a hypothetical question?
Best regards,
Wolfgang Denk
On 22/04/11 22:46, Wolfgang Denk wrote:
Dear Graeme Russ,
In message 4DB15DD7.8050404@gmail.com you wrote:
I think it wouldbe even better if we could push our changes back into the "mainline" version of checkpatch, so that the U-Boot specific behaviour can beenabled by a command line option (checkpatch --uboot ?).
Forking is not so preferrable here, I think.
I agree, but if the Linux guys won't accept patches for U-Boot specific semantics, what choice do we have?
Did you ask him, and he refused, or is this just a hypothetical question?
I have now asked on LKML - Let's see what unfolds...
Regards,
Graeme
participants (8)
-
Albert ARIBAUD -
Andreas Pretzsch -
Detlev Zundel -
Eric Cooper -
Fabian Cenedese -
Graeme Russ -
Scott Wood -
Wolfgang Denk