[RFC] checkpatch.py

Stefano Mioli jstevie at gmail.com
Sat Jul 4 07:37:22 CDT 2009


Giorgos Keramidas wrote:

>> It's still very simple, but Matt said he would like to have such a
>> tool in contrib, so here it is.
> 
> It's a very good start indeed, thanks! :)

Thank you for such a useful review.

>> - leading tabs
>> - trailing whitespaces
>> - DOS-style line endings
>> - lines longer than 80 characters
> 
> These should probably be encoded in a dictionary or array of tuples, or
> even a small class of checks, so we can write as many checks as possible
> by simply adding new regexps to a vector instead of having to modify the
> checking code?

That makes *very* much sense.
I even thought about moving regexes initialization out of the way and make it 
more dynamic and maintainable, but eventually ended up leaving the code as it was.

>     def initchecks():

[snip]

>     def checkpatch(fname):

[snip]

> Since you are handling already one file at a time in checkpatch() it may
> be worth supporting more than one input file, so one can type in a
> directory full of patch files:
> 
>     yourscript.py *.patch
> 
> How about something like?
> 
>     progname = os.path.basename(sys.argv[0])
>     if len(sys.argv) == 1:
>         print "usage: %s patchfile [patchfile ...]" % progname
>         sys.exit(1)
>     for fn in sys.argv[1:]:
>         checkpatch(fn)

I'll definitely do something like this.
Of course I'll need to avoid the recreation of the list of regexes for each
file.

I'm thinking about moving the call to initchecks() from checkpatch() to right
before the loop over the arguments, and thus modify checkpatch()'s signature
to also take a list of checks. I think it's also better from a logical point of
view.

Would that be the right way?

Giorgos, thank you for your helpful suggestions and code.
I'm going to whip up a new patch soon and see if I am able to do it right.



More information about the Mercurial-devel mailing list