| Message ID | 1592150583-178826-1-git-send-email-roid@mellanox.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series | [ovs-dev] checkpatch: Add argument to skip gerrit change id check | expand |
On 2020-06-14 7:03 PM, Roi Dayan wrote: > This arg can be used internally by groups using gerrit for code reviews. > > Signed-off-by: Roi Dayan <roid@mellanox.com> > --- > utilities/checkpatch.py | 9 +++++++-- > 1 file changed, 7 insertions(+), 2 deletions(-) > > diff --git a/utilities/checkpatch.py b/utilities/checkpatch.py > index fc9e20bf1b5f..78c8c9ce49c7 100755 > --- a/utilities/checkpatch.py > +++ b/utilities/checkpatch.py > @@ -182,6 +182,7 @@ __regex_if_macros = re.compile(r'^ +(%s) \([\S]([\s\S]+[\S])*\) { +\\' % > > skip_leading_whitespace_check = False > skip_trailing_whitespace_check = False > +skip_gerrit_change_id_check = False > skip_block_whitespace_check = False > skip_signoff_check = False > > @@ -814,7 +815,7 @@ def ovs_checkpatch_parse(text, filename, author=None, committer=None): > elif is_co_author.match(line): > m = is_co_author.match(line) > co_authors.append(m.group(2)) > - elif is_gerrit_change_id.match(line): > + elif is_gerrit_change_id.match(line) and not skip_gerrit_change_id_check: > print_error( > "Remove Gerrit Change-Id's before submitting upstream.") > print("%d: %s\n" % (lineno, line)) > @@ -885,7 +886,8 @@ Check options: > -s|--skip-signoff-lines Tolerate missing Signed-off-by line > -S|--spellcheck Check C comments and commit-message for possible > spelling mistakes > --t|--skip-trailing-whitespace Skips the trailing whitespace test""" > +-t|--skip-trailing-whitespace Skips the trailing whitespace test > + --skip-gerrit-change-id Skips the gerrit change id test""" > % sys.argv[0]) > > > @@ -942,6 +944,7 @@ if __name__ == '__main__': > "skip-leading-whitespace", > "skip-signoff-lines", > "skip-trailing-whitespace", > + "skip-gerrit-change-id", > "spellcheck", > "quiet"]) > except: > @@ -960,6 +963,8 @@ if __name__ == '__main__': > skip_signoff_check = True > elif o in ("-t", "--skip-trailing-whitespace"): > skip_trailing_whitespace_check = True > + elif o in ("--skip-gerrit-change-id"): > + skip_gerrit_change_id_check = True > elif o in ("-f", "--check-file"): > checking_file = True > elif o in ("-S", "--spellcheck"): > Hi, Any comments on this? Thanks, Roi
On Tue, Jun 16, 2020 at 04:14:23PM +0300, Roi Dayan wrote: > > > On 2020-06-14 7:03 PM, Roi Dayan wrote: > > This arg can be used internally by groups using gerrit for code reviews. > > > > Signed-off-by: Roi Dayan <roid@mellanox.com> > > --- > > utilities/checkpatch.py | 9 +++++++-- > > 1 file changed, 7 insertions(+), 2 deletions(-) > > > > diff --git a/utilities/checkpatch.py b/utilities/checkpatch.py > > index fc9e20bf1b5f..78c8c9ce49c7 100755 > > --- a/utilities/checkpatch.py > > +++ b/utilities/checkpatch.py > > @@ -182,6 +182,7 @@ __regex_if_macros = re.compile(r'^ +(%s) \([\S]([\s\S]+[\S])*\) { +\\' % > > > > skip_leading_whitespace_check = False > > skip_trailing_whitespace_check = False > > +skip_gerrit_change_id_check = False > > skip_block_whitespace_check = False > > skip_signoff_check = False > > > > @@ -814,7 +815,7 @@ def ovs_checkpatch_parse(text, filename, author=None, committer=None): > > elif is_co_author.match(line): > > m = is_co_author.match(line) > > co_authors.append(m.group(2)) > > - elif is_gerrit_change_id.match(line): > > + elif is_gerrit_change_id.match(line) and not skip_gerrit_change_id_check: > > print_error( > > "Remove Gerrit Change-Id's before submitting upstream.") > > print("%d: %s\n" % (lineno, line)) > > @@ -885,7 +886,8 @@ Check options: > > -s|--skip-signoff-lines Tolerate missing Signed-off-by line > > -S|--spellcheck Check C comments and commit-message for possible > > spelling mistakes > > --t|--skip-trailing-whitespace Skips the trailing whitespace test""" > > +-t|--skip-trailing-whitespace Skips the trailing whitespace test > > + --skip-gerrit-change-id Skips the gerrit change id test""" > > % sys.argv[0]) > > > > > > @@ -942,6 +944,7 @@ if __name__ == '__main__': > > "skip-leading-whitespace", > > "skip-signoff-lines", > > "skip-trailing-whitespace", > > + "skip-gerrit-change-id", > > "spellcheck", > > "quiet"]) > > except: > > @@ -960,6 +963,8 @@ if __name__ == '__main__': > > skip_signoff_check = True > > elif o in ("-t", "--skip-trailing-whitespace"): > > skip_trailing_whitespace_check = True > > + elif o in ("--skip-gerrit-change-id"): > > + skip_gerrit_change_id_check = True > > elif o in ("-f", "--check-file"): > > checking_file = True > > elif o in ("-S", "--spellcheck"): > > > > Hi, > > Any comments on this? Reviewed-by: Simon Horman <simon.horman@netronome.com>
On 2020-06-16 5:05 PM, Simon Horman wrote: > On Tue, Jun 16, 2020 at 04:14:23PM +0300, Roi Dayan wrote: >> >> >> On 2020-06-14 7:03 PM, Roi Dayan wrote: >>> This arg can be used internally by groups using gerrit for code reviews. >>> >>> Signed-off-by: Roi Dayan <roid@mellanox.com> >>> --- >>> utilities/checkpatch.py | 9 +++++++-- >>> 1 file changed, 7 insertions(+), 2 deletions(-) >>> >>> diff --git a/utilities/checkpatch.py b/utilities/checkpatch.py >>> index fc9e20bf1b5f..78c8c9ce49c7 100755 >>> --- a/utilities/checkpatch.py >>> +++ b/utilities/checkpatch.py >>> @@ -182,6 +182,7 @@ __regex_if_macros = re.compile(r'^ +(%s) \([\S]([\s\S]+[\S])*\) { +\\' % >>> >>> skip_leading_whitespace_check = False >>> skip_trailing_whitespace_check = False >>> +skip_gerrit_change_id_check = False >>> skip_block_whitespace_check = False >>> skip_signoff_check = False >>> >>> @@ -814,7 +815,7 @@ def ovs_checkpatch_parse(text, filename, author=None, committer=None): >>> elif is_co_author.match(line): >>> m = is_co_author.match(line) >>> co_authors.append(m.group(2)) >>> - elif is_gerrit_change_id.match(line): >>> + elif is_gerrit_change_id.match(line) and not skip_gerrit_change_id_check: >>> print_error( >>> "Remove Gerrit Change-Id's before submitting upstream.") >>> print("%d: %s\n" % (lineno, line)) >>> @@ -885,7 +886,8 @@ Check options: >>> -s|--skip-signoff-lines Tolerate missing Signed-off-by line >>> -S|--spellcheck Check C comments and commit-message for possible >>> spelling mistakes >>> --t|--skip-trailing-whitespace Skips the trailing whitespace test""" >>> +-t|--skip-trailing-whitespace Skips the trailing whitespace test >>> + --skip-gerrit-change-id Skips the gerrit change id test""" >>> % sys.argv[0]) >>> >>> >>> @@ -942,6 +944,7 @@ if __name__ == '__main__': >>> "skip-leading-whitespace", >>> "skip-signoff-lines", >>> "skip-trailing-whitespace", >>> + "skip-gerrit-change-id", >>> "spellcheck", >>> "quiet"]) >>> except: >>> @@ -960,6 +963,8 @@ if __name__ == '__main__': >>> skip_signoff_check = True >>> elif o in ("-t", "--skip-trailing-whitespace"): >>> skip_trailing_whitespace_check = True >>> + elif o in ("--skip-gerrit-change-id"): >>> + skip_gerrit_change_id_check = True >>> elif o in ("-f", "--check-file"): >>> checking_file = True >>> elif o in ("-S", "--spellcheck"): >>> >> >> Hi, >> >> Any comments on this? > > Reviewed-by: Simon Horman <simon.horman@netronome.com> > thanks. there are no other comments. can we merge this?
On Sun, Jun 14, 2020 at 07:03:03PM +0300, Roi Dayan wrote: > This arg can be used internally by groups using gerrit for code reviews. The patch looks good, but there is the condition line that goes up to column 85. It's the only line beyond column 79 in that file. Sorry the nitpicking, but could you please fix that? fbl > > Signed-off-by: Roi Dayan <roid@mellanox.com> > --- > utilities/checkpatch.py | 9 +++++++-- > 1 file changed, 7 insertions(+), 2 deletions(-) > > diff --git a/utilities/checkpatch.py b/utilities/checkpatch.py > index fc9e20bf1b5f..78c8c9ce49c7 100755 > --- a/utilities/checkpatch.py > +++ b/utilities/checkpatch.py > @@ -182,6 +182,7 @@ __regex_if_macros = re.compile(r'^ +(%s) \([\S]([\s\S]+[\S])*\) { +\\' % > > skip_leading_whitespace_check = False > skip_trailing_whitespace_check = False > +skip_gerrit_change_id_check = False > skip_block_whitespace_check = False > skip_signoff_check = False > > @@ -814,7 +815,7 @@ def ovs_checkpatch_parse(text, filename, author=None, committer=None): > elif is_co_author.match(line): > m = is_co_author.match(line) > co_authors.append(m.group(2)) > - elif is_gerrit_change_id.match(line): > + elif is_gerrit_change_id.match(line) and not skip_gerrit_change_id_check: > print_error( > "Remove Gerrit Change-Id's before submitting upstream.") > print("%d: %s\n" % (lineno, line)) > @@ -885,7 +886,8 @@ Check options: > -s|--skip-signoff-lines Tolerate missing Signed-off-by line > -S|--spellcheck Check C comments and commit-message for possible > spelling mistakes > --t|--skip-trailing-whitespace Skips the trailing whitespace test""" > +-t|--skip-trailing-whitespace Skips the trailing whitespace test > + --skip-gerrit-change-id Skips the gerrit change id test""" > % sys.argv[0]) > > > @@ -942,6 +944,7 @@ if __name__ == '__main__': > "skip-leading-whitespace", > "skip-signoff-lines", > "skip-trailing-whitespace", > + "skip-gerrit-change-id", > "spellcheck", > "quiet"]) > except: > @@ -960,6 +963,8 @@ if __name__ == '__main__': > skip_signoff_check = True > elif o in ("-t", "--skip-trailing-whitespace"): > skip_trailing_whitespace_check = True > + elif o in ("--skip-gerrit-change-id"): > + skip_gerrit_change_id_check = True > elif o in ("-f", "--check-file"): > checking_file = True > elif o in ("-S", "--spellcheck"): > -- > 2.8.4 > > _______________________________________________ > dev mailing list > dev@openvswitch.org > https://mail.openvswitch.org/mailman/listinfo/ovs-dev
On 7/9/20 5:09 PM, Flavio Leitner wrote: > On Sun, Jun 14, 2020 at 07:03:03PM +0300, Roi Dayan wrote: >> This arg can be used internally by groups using gerrit for code reviews. > > The patch looks good, but there is the condition line > that goes up to column 85. It's the only line beyond > column 79 in that file. > > Sorry the nitpicking, but could you please fix that? It's not a nitpicking, it's a PEP8 violation that breaks the build: https://travis-ci.org/github/ovsrobot/ovs/jobs/698255640#L2702 > > fbl > >> >> Signed-off-by: Roi Dayan <roid@mellanox.com> >> --- >> utilities/checkpatch.py | 9 +++++++-- >> 1 file changed, 7 insertions(+), 2 deletions(-) >> >> diff --git a/utilities/checkpatch.py b/utilities/checkpatch.py >> index fc9e20bf1b5f..78c8c9ce49c7 100755 >> --- a/utilities/checkpatch.py >> +++ b/utilities/checkpatch.py >> @@ -182,6 +182,7 @@ __regex_if_macros = re.compile(r'^ +(%s) \([\S]([\s\S]+[\S])*\) { +\\' % >> >> skip_leading_whitespace_check = False >> skip_trailing_whitespace_check = False >> +skip_gerrit_change_id_check = False >> skip_block_whitespace_check = False >> skip_signoff_check = False >> >> @@ -814,7 +815,7 @@ def ovs_checkpatch_parse(text, filename, author=None, committer=None): >> elif is_co_author.match(line): >> m = is_co_author.match(line) >> co_authors.append(m.group(2)) >> - elif is_gerrit_change_id.match(line): >> + elif is_gerrit_change_id.match(line) and not skip_gerrit_change_id_check: >> print_error( >> "Remove Gerrit Change-Id's before submitting upstream.") >> print("%d: %s\n" % (lineno, line)) >> @@ -885,7 +886,8 @@ Check options: >> -s|--skip-signoff-lines Tolerate missing Signed-off-by line >> -S|--spellcheck Check C comments and commit-message for possible >> spelling mistakes >> --t|--skip-trailing-whitespace Skips the trailing whitespace test""" >> +-t|--skip-trailing-whitespace Skips the trailing whitespace test >> + --skip-gerrit-change-id Skips the gerrit change id test""" >> % sys.argv[0]) >> >> >> @@ -942,6 +944,7 @@ if __name__ == '__main__': >> "skip-leading-whitespace", >> "skip-signoff-lines", >> "skip-trailing-whitespace", >> + "skip-gerrit-change-id", >> "spellcheck", >> "quiet"]) >> except: >> @@ -960,6 +963,8 @@ if __name__ == '__main__': >> skip_signoff_check = True >> elif o in ("-t", "--skip-trailing-whitespace"): >> skip_trailing_whitespace_check = True >> + elif o in ("--skip-gerrit-change-id"): >> + skip_gerrit_change_id_check = True >> elif o in ("-f", "--check-file"): >> checking_file = True >> elif o in ("-S", "--spellcheck"): >> -- >> 2.8.4 >> >> _______________________________________________ >> dev mailing list >> dev@openvswitch.org >> https://mail.openvswitch.org/mailman/listinfo/ovs-dev >
On 2020-07-09 9:30 PM, Ilya Maximets wrote: > On 7/9/20 5:09 PM, Flavio Leitner wrote: >> On Sun, Jun 14, 2020 at 07:03:03PM +0300, Roi Dayan wrote: >>> This arg can be used internally by groups using gerrit for code reviews. >> >> The patch looks good, but there is the condition line >> that goes up to column 85. It's the only line beyond >> column 79 in that file. >> >> Sorry the nitpicking, but could you please fix that? > > It's not a nitpicking, it's a PEP8 violation that breaks the build: > https://eur03.safelinks.protection.outlook.com/?url=https%3A%2F%2Ftravis-ci.org%2Fgithub%2Fovsrobot%2Fovs%2Fjobs%2F698255640%23L2702&data=02%7C01%7Croid%40mellanox.com%7C3ad7cba04fc64baad9be08d824361c1e%7Ca652971c7d2e4d9ba6a4d149256f461b%7C0%7C0%7C637299162109839762&sdata=I0Y3LghCrqsgnNC4Efz0iNJ1MYGo8xFC4fx0U%2FEJGKI%3D&reserved=0 > ok. i saw Ben sent v2 so i'll update that one and send v3. >> >> fbl >> >>> >>> Signed-off-by: Roi Dayan <roid@mellanox.com> >>> --- >>> utilities/checkpatch.py | 9 +++++++-- >>> 1 file changed, 7 insertions(+), 2 deletions(-) >>> >>> diff --git a/utilities/checkpatch.py b/utilities/checkpatch.py >>> index fc9e20bf1b5f..78c8c9ce49c7 100755 >>> --- a/utilities/checkpatch.py >>> +++ b/utilities/checkpatch.py >>> @@ -182,6 +182,7 @@ __regex_if_macros = re.compile(r'^ +(%s) \([\S]([\s\S]+[\S])*\) { +\\' % >>> >>> skip_leading_whitespace_check = False >>> skip_trailing_whitespace_check = False >>> +skip_gerrit_change_id_check = False >>> skip_block_whitespace_check = False >>> skip_signoff_check = False >>> >>> @@ -814,7 +815,7 @@ def ovs_checkpatch_parse(text, filename, author=None, committer=None): >>> elif is_co_author.match(line): >>> m = is_co_author.match(line) >>> co_authors.append(m.group(2)) >>> - elif is_gerrit_change_id.match(line): >>> + elif is_gerrit_change_id.match(line) and not skip_gerrit_change_id_check: >>> print_error( >>> "Remove Gerrit Change-Id's before submitting upstream.") >>> print("%d: %s\n" % (lineno, line)) >>> @@ -885,7 +886,8 @@ Check options: >>> -s|--skip-signoff-lines Tolerate missing Signed-off-by line >>> -S|--spellcheck Check C comments and commit-message for possible >>> spelling mistakes >>> --t|--skip-trailing-whitespace Skips the trailing whitespace test""" >>> +-t|--skip-trailing-whitespace Skips the trailing whitespace test >>> + --skip-gerrit-change-id Skips the gerrit change id test""" >>> % sys.argv[0]) >>> >>> >>> @@ -942,6 +944,7 @@ if __name__ == '__main__': >>> "skip-leading-whitespace", >>> "skip-signoff-lines", >>> "skip-trailing-whitespace", >>> + "skip-gerrit-change-id", >>> "spellcheck", >>> "quiet"]) >>> except: >>> @@ -960,6 +963,8 @@ if __name__ == '__main__': >>> skip_signoff_check = True >>> elif o in ("-t", "--skip-trailing-whitespace"): >>> skip_trailing_whitespace_check = True >>> + elif o in ("--skip-gerrit-change-id"): >>> + skip_gerrit_change_id_check = True >>> elif o in ("-f", "--check-file"): >>> checking_file = True >>> elif o in ("-S", "--spellcheck"): >>> -- >>> 2.8.4 >>> >>> _______________________________________________ >>> dev mailing list >>> dev@openvswitch.org >>> https://eur03.safelinks.protection.outlook.com/?url=https%3A%2F%2Fmail.openvswitch.org%2Fmailman%2Flistinfo%2Fovs-dev&data=02%7C01%7Croid%40mellanox.com%7C3ad7cba04fc64baad9be08d824361c1e%7Ca652971c7d2e4d9ba6a4d149256f461b%7C0%7C0%7C637299162109839762&sdata=vjxbsymqIz67PcJvFOTUrHPP7uKiEPW18J84emlgPao%3D&reserved=0 >> >
diff --git a/utilities/checkpatch.py b/utilities/checkpatch.py index fc9e20bf1b5f..78c8c9ce49c7 100755 --- a/utilities/checkpatch.py +++ b/utilities/checkpatch.py @@ -182,6 +182,7 @@ __regex_if_macros = re.compile(r'^ +(%s) \([\S]([\s\S]+[\S])*\) { +\\' % skip_leading_whitespace_check = False skip_trailing_whitespace_check = False +skip_gerrit_change_id_check = False skip_block_whitespace_check = False skip_signoff_check = False @@ -814,7 +815,7 @@ def ovs_checkpatch_parse(text, filename, author=None, committer=None): elif is_co_author.match(line): m = is_co_author.match(line) co_authors.append(m.group(2)) - elif is_gerrit_change_id.match(line): + elif is_gerrit_change_id.match(line) and not skip_gerrit_change_id_check: print_error( "Remove Gerrit Change-Id's before submitting upstream.") print("%d: %s\n" % (lineno, line)) @@ -885,7 +886,8 @@ Check options: -s|--skip-signoff-lines Tolerate missing Signed-off-by line -S|--spellcheck Check C comments and commit-message for possible spelling mistakes --t|--skip-trailing-whitespace Skips the trailing whitespace test""" +-t|--skip-trailing-whitespace Skips the trailing whitespace test + --skip-gerrit-change-id Skips the gerrit change id test""" % sys.argv[0]) @@ -942,6 +944,7 @@ if __name__ == '__main__': "skip-leading-whitespace", "skip-signoff-lines", "skip-trailing-whitespace", + "skip-gerrit-change-id", "spellcheck", "quiet"]) except: @@ -960,6 +963,8 @@ if __name__ == '__main__': skip_signoff_check = True elif o in ("-t", "--skip-trailing-whitespace"): skip_trailing_whitespace_check = True + elif o in ("--skip-gerrit-change-id"): + skip_gerrit_change_id_check = True elif o in ("-f", "--check-file"): checking_file = True elif o in ("-S", "--spellcheck"):
This arg can be used internally by groups using gerrit for code reviews. Signed-off-by: Roi Dayan <roid@mellanox.com> --- utilities/checkpatch.py | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-)