Russell Bryant <russ...@ovn.org> writes:
> On Thu, Mar 24, 2016 at 4:12 PM, Aaron Conole <acon...@redhat.com> wrote:
>
>> Most projects have a checkpatch facility, which can be used as a pre-commit
>> sanity check. This introduces such a mechanism to the Open vSwitch project
>> to catch some of the more silly formatting mistakes which can occur. It is
>> not meant to replace good code review practices, but it can help eliminate
>> the silly code review issues which get added.
>>
>> Suggested-by: Mauricio Vásquez <mauricio.vasquezber...@studenti.polito.it>
>> Signed-off-by: Aaron Conole <acon...@redhat.com>
>> ---
>> v2:
>> - Moved to the utilities directory
>> - Fixed up according to flake8
>> - Added an ignore for leading tabs with .mk file modifications
>> - hooked up the return values
>>
>> CONTRIBUTING.md | 4 ++
>> utilities/automake.mk | 3 +-
>> utilities/checkpatch.py | 158
>> ++++++++++++++++++++++++++++++++++++++++++++++++
>> 3 files changed, 164 insertions(+), 1 deletion(-)
>> create mode 100644 utilities/checkpatch.py
>>
>
> "make" still fails for me. See the incremental diff below. Part of it is
> that I have an additional flake8 plugin installed called "hacking" that
Okay, I'll install that.
> caught the print function compatibility issue. Even without that, "make"
> failed since the new file wasn't lised in EXTRA_DIST.
D'oh! I did make flake8-check, but I'll do a make check before I submit
v3.
> It would be nice to have this marked as executable.
Sure thing.
> Some help text when run with -h/--help would be nice, as it wasn't obvious
> how to use it. It should also give a helpful error if you don't provide
> expected arguments.
Okay, I'll add optargs support.
> For example, I just guessed at how to use it and got it wrong.
>
> $ git show | python utilities/checkpatch.py
> Traceback (most recent call last):
> File "utilities/checkpatch.py", line 160, in <module>
> sys.exit(ovs_checkpatch_file(sys.argv[1]))
> IndexError: list index out of range
>
> Support for running it that way would be nice, but not required. :-)
You ask for it... :)
> diff --git a/utilities/automake.mk b/utilities/automake.mk
> index 8062d8b..5be9c8c 100644
> --- a/utilities/automake.mk
> +++ b/utilities/automake.mk
> @@ -56,7 +56,8 @@ EXTRA_DIST += \
> utilities/ovs-test.in \
> utilities/ovs-vlan-test.in \
> utilities/ovs-vsctl-bashcomp.bash \
> - utilities/qemu-wrap.py
> + utilities/qemu-wrap.py \
> + utilities/checkpatch.py
> MAN_ROOTS += \
> utilities/ovs-appctl.8.in \
> utilities/ovs-testcontroller.8.in \
> diff --git a/utilities/checkpatch.py b/utilities/checkpatch.py
> index f9caeae..1687187 100644
> --- a/utilities/checkpatch.py
> +++ b/utilities/checkpatch.py
> @@ -12,6 +12,8 @@
> # See the License for the specific language governing permissions and
> # limitations under the License.
>
> +from __future__ import print_function
> +
> import re
> import sys
> import email
> @@ -23,9 +25,9 @@ __warnings = 0
> def print_error(message, lineno=None):
> global __errors
> if lineno is not None:
> - print "E(%d): %s" % (lineno, message)
> + print("E(%d): %s" % (lineno, message))
> else:
> - print "E: %s" % (message)
> + print("E: %s" % (message))
>
> __errors = __errors + 1
>
> @@ -33,9 +35,9 @@ def print_error(message, lineno=None):
> def print_warning(message, lineno=None):
> global __warnings
> if lineno:
> - print "W(%d): %s" % (lineno, message)
> + print("W(%d): %s" % (lineno, message))
> else:
> - print "W: %s" % (message)
> + print("W: %s" % (message))
>
> __warnings = __warnings + 1
>
> @@ -136,7 +138,7 @@ def ovs_checkpatch_parse(text):
> print_warning("Improper whitespace around control block",
> lineno)
> if print_line:
> - print line
> + print(line)
> if __errors or __warnings:
> return -1
> return 0
> _______________________________________________
> dev mailing list
> dev@openvswitch.org
> http://openvswitch.org/mailman/listinfo/dev
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
