| Message ID | 20230623121233.315243-1-dceara@redhat.com |
|---|---|
| State | Accepted |
| Headers | show |
| Series | [ovs-dev] checkpatch: Ignore yml files when checking line lenghts. | expand |
| Context | Check | Description |
|---|---|---|
| ovsrobot/apply-robot | success | apply and check: success |
| ovsrobot/github-robot-_Build_and_Test | success | github build: passed |
| ovsrobot/intel-ovs-compilation | success | test: success |
On 23 Jun 2023, at 14:12, Dumitru Ceara wrote: > As far as I can tell they're used mostly for CI job definitions and > these tend to result in long lines. > > Reported-at: https://mail.openvswitch.org/pipermail/ovs-dev/2023-June/405796.html > Suggested-by: Aaron Conole <aconole@redhat.com> > Signed-off-by: Dumitru Ceara <dceara@redhat.com> Looks good to me. Acked-by: Eelco Chaudron <echaudro@redhat.com>
On 23 Jun 2023, at 14:22, Eelco Chaudron wrote: > On 23 Jun 2023, at 14:12, Dumitru Ceara wrote: > >> As far as I can tell they're used mostly for CI job definitions and >> these tend to result in long lines. >> >> Reported-at: https://mail.openvswitch.org/pipermail/ovs-dev/2023-June/405796.html >> Suggested-by: Aaron Conole <aconole@redhat.com> >> Signed-off-by: Dumitru Ceara <dceara@redhat.com> > > Looks good to me. > > Acked-by: Eelco Chaudron <echaudro@redhat.com> Except that ‘lenghts’ in the subject is spelled wrong :)
On 6/23/23 14:23, Eelco Chaudron wrote: > > > On 23 Jun 2023, at 14:22, Eelco Chaudron wrote: > >> On 23 Jun 2023, at 14:12, Dumitru Ceara wrote: >> >>> As far as I can tell they're used mostly for CI job definitions and >>> these tend to result in long lines. >>> >>> Reported-at: https://mail.openvswitch.org/pipermail/ovs-dev/2023-June/405796.html >>> Suggested-by: Aaron Conole <aconole@redhat.com> >>> Signed-off-by: Dumitru Ceara <dceara@redhat.com> >> >> Looks good to me. >> >> Acked-by: Eelco Chaudron <echaudro@redhat.com> > > Except that ‘lenghts’ in the subject is spelled wrong :) > Oh, oops. I can post a v2 if needed, just let me know please. Thanks, Dumitru
On 23 Jun 2023, at 14:33, Dumitru Ceara wrote: > On 6/23/23 14:23, Eelco Chaudron wrote: >> >> >> On 23 Jun 2023, at 14:22, Eelco Chaudron wrote: >> >>> On 23 Jun 2023, at 14:12, Dumitru Ceara wrote: >>> >>>> As far as I can tell they're used mostly for CI job definitions and >>>> these tend to result in long lines. >>>> >>>> Reported-at: https://mail.openvswitch.org/pipermail/ovs-dev/2023-June/405796.html >>>> Suggested-by: Aaron Conole <aconole@redhat.com> >>>> Signed-off-by: Dumitru Ceara <dceara@redhat.com> >>> >>> Looks good to me. >>> >>> Acked-by: Eelco Chaudron <echaudro@redhat.com> >> >> Except that ‘lenghts’ in the subject is spelled wrong :) >> > > Oh, oops. I can post a v2 if needed, just let me know please. I guess the committer can change that :)
Dumitru Ceara <dceara@redhat.com> writes: > As far as I can tell they're used mostly for CI job definitions and > these tend to result in long lines. > > Reported-at: https://mail.openvswitch.org/pipermail/ovs-dev/2023-June/405796.html > Suggested-by: Aaron Conole <aconole@redhat.com> > Signed-off-by: Dumitru Ceara <dceara@redhat.com> > --- Acked-by: Aaron Conole <aconole@redhat.com>
On 6/23/23 14:12, Dumitru Ceara wrote: > As far as I can tell they're used mostly for CI job definitions and > these tend to result in long lines. Why not just wrap them? AFAIK, syntax in most CI systems allows line wrapping. Not a strong opinion though, just curious. Best regards, Ilya Maximets. > > Reported-at: https://mail.openvswitch.org/pipermail/ovs-dev/2023-June/405796.html > Suggested-by: Aaron Conole <aconole@redhat.com> > Signed-off-by: Dumitru Ceara <dceara@redhat.com> > --- > utilities/checkpatch.py | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/utilities/checkpatch.py b/utilities/checkpatch.py > index 0d30b71b5b7f..64f0efeb474e 100755 > --- a/utilities/checkpatch.py > +++ b/utilities/checkpatch.py > @@ -195,7 +195,7 @@ skip_signoff_check = False > # > # Python isn't checked as flake8 performs these checks during build. > line_length_ignore_list = re.compile( > - r'\.(am|at|etc|in|m4|mk|patch|py)$|^debian/.*$') > + r'\.(am|at|etc|in|m4|mk|patch|py|yml)$|^debian/.*$') > > # Don't enforce a requirement that leading whitespace be all spaces on > # files that include these characters in their name, since these kinds
On 6/27/23 22:12, Ilya Maximets wrote: > On 6/23/23 14:12, Dumitru Ceara wrote: >> As far as I can tell they're used mostly for CI job definitions and >> these tend to result in long lines. > > Why not just wrap them? AFAIK, syntax in most CI systems allows > line wrapping. > > Not a strong opinion though, just curious. > I found it less readable in some cases, e.g.: https://github.com/openvswitch/ovs/blob/master/appveyor.yml#L39-L55 https://github.com/ovn-org/ovn/blob/main/.github/workflows/test.yml#L32-L55 https://github.com/ovn-org/ovn/blob/main/.github/workflows/ovn-kubernetes.yml#L86 It's a personal opinion, of course. Thanks, Dumitru > Best regards, Ilya Maximets. > >> >> Reported-at: https://mail.openvswitch.org/pipermail/ovs-dev/2023-June/405796.html >> Suggested-by: Aaron Conole <aconole@redhat.com> >> Signed-off-by: Dumitru Ceara <dceara@redhat.com> >> --- >> utilities/checkpatch.py | 2 +- >> 1 file changed, 1 insertion(+), 1 deletion(-) >> >> diff --git a/utilities/checkpatch.py b/utilities/checkpatch.py >> index 0d30b71b5b7f..64f0efeb474e 100755 >> --- a/utilities/checkpatch.py >> +++ b/utilities/checkpatch.py >> @@ -195,7 +195,7 @@ skip_signoff_check = False >> # >> # Python isn't checked as flake8 performs these checks during build. >> line_length_ignore_list = re.compile( >> - r'\.(am|at|etc|in|m4|mk|patch|py)$|^debian/.*$') >> + r'\.(am|at|etc|in|m4|mk|patch|py|yml)$|^debian/.*$') >> >> # Don't enforce a requirement that leading whitespace be all spaces on >> # files that include these characters in their name, since these kinds >
On 6/28/23 10:18, Dumitru Ceara wrote: > On 6/27/23 22:12, Ilya Maximets wrote: >> On 6/23/23 14:12, Dumitru Ceara wrote: >>> As far as I can tell they're used mostly for CI job definitions and >>> these tend to result in long lines. >> >> Why not just wrap them? AFAIK, syntax in most CI systems allows >> line wrapping. >> >> Not a strong opinion though, just curious. >> > > I found it less readable in some cases, e.g.: > > https://github.com/openvswitch/ovs/blob/master/appveyor.yml#L39-L55 Yeah, I don't have a good alternative for that. > > https://github.com/ovn-org/ovn/blob/main/.github/workflows/test.yml#L32-L55 These do not look good, IMHO. So, the reminder that these are not great, in a form of a line length warning, is kind of OK. > > https://github.com/ovn-org/ovn/blob/main/.github/workflows/ovn-kubernetes.yml#L86 And this doesn't fit into a screen even in a browser window for me. So, I'd not call it readable. :) I literally can't read it, I have to scroll. I agree with the appveyor example though, so applied. Thanks! Best regards, Ilya Maximets. > > It's a personal opinion, of course. > > Thanks, > Dumitru > >> Best regards, Ilya Maximets. >> >>> >>> Reported-at: https://mail.openvswitch.org/pipermail/ovs-dev/2023-June/405796.html >>> Suggested-by: Aaron Conole <aconole@redhat.com> >>> Signed-off-by: Dumitru Ceara <dceara@redhat.com> >>> --- >>> utilities/checkpatch.py | 2 +- >>> 1 file changed, 1 insertion(+), 1 deletion(-) >>> >>> diff --git a/utilities/checkpatch.py b/utilities/checkpatch.py >>> index 0d30b71b5b7f..64f0efeb474e 100755 >>> --- a/utilities/checkpatch.py >>> +++ b/utilities/checkpatch.py >>> @@ -195,7 +195,7 @@ skip_signoff_check = False >>> # >>> # Python isn't checked as flake8 performs these checks during build. >>> line_length_ignore_list = re.compile( >>> - r'\.(am|at|etc|in|m4|mk|patch|py)$|^debian/.*$') >>> + r'\.(am|at|etc|in|m4|mk|patch|py|yml)$|^debian/.*$') >>> >>> # Don't enforce a requirement that leading whitespace be all spaces on >>> # files that include these characters in their name, since these kinds >> >
On 6/28/23 13:19, Ilya Maximets wrote: > On 6/28/23 10:18, Dumitru Ceara wrote: >> On 6/27/23 22:12, Ilya Maximets wrote: >>> On 6/23/23 14:12, Dumitru Ceara wrote: >>>> As far as I can tell they're used mostly for CI job definitions and >>>> these tend to result in long lines. >>> >>> Why not just wrap them? AFAIK, syntax in most CI systems allows >>> line wrapping. >>> >>> Not a strong opinion though, just curious. >>> >> >> I found it less readable in some cases, e.g.: >> >> https://github.com/openvswitch/ovs/blob/master/appveyor.yml#L39-L55 > > Yeah, I don't have a good alternative for that. > >> >> https://github.com/ovn-org/ovn/blob/main/.github/workflows/test.yml#L32-L55 > > These do not look good, IMHO. So, the reminder that these are not great, > in a form of a line length warning, is kind of OK. > >> >> https://github.com/ovn-org/ovn/blob/main/.github/workflows/ovn-kubernetes.yml#L86 > > And this doesn't fit into a screen even in a browser window for me. > So, I'd not call it readable. :) I literally can't read it, I have > to scroll. > Maybe we should fix it at some point. > > I agree with the appveyor example though, so applied. Thanks! > Thanks! > Best regards, Ilya Maximets. > >> >> It's a personal opinion, of course. >> >> Thanks, >> Dumitru >> >>> Best regards, Ilya Maximets. >>> >>>> >>>> Reported-at: https://mail.openvswitch.org/pipermail/ovs-dev/2023-June/405796.html >>>> Suggested-by: Aaron Conole <aconole@redhat.com> >>>> Signed-off-by: Dumitru Ceara <dceara@redhat.com> >>>> --- >>>> utilities/checkpatch.py | 2 +- >>>> 1 file changed, 1 insertion(+), 1 deletion(-) >>>> >>>> diff --git a/utilities/checkpatch.py b/utilities/checkpatch.py >>>> index 0d30b71b5b7f..64f0efeb474e 100755 >>>> --- a/utilities/checkpatch.py >>>> +++ b/utilities/checkpatch.py >>>> @@ -195,7 +195,7 @@ skip_signoff_check = False >>>> # >>>> # Python isn't checked as flake8 performs these checks during build. >>>> line_length_ignore_list = re.compile( >>>> - r'\.(am|at|etc|in|m4|mk|patch|py)$|^debian/.*$') >>>> + r'\.(am|at|etc|in|m4|mk|patch|py|yml)$|^debian/.*$') >>>> >>>> # Don't enforce a requirement that leading whitespace be all spaces on >>>> # files that include these characters in their name, since these kinds >>> >> >
diff --git a/utilities/checkpatch.py b/utilities/checkpatch.py index 0d30b71b5b7f..64f0efeb474e 100755 --- a/utilities/checkpatch.py +++ b/utilities/checkpatch.py @@ -195,7 +195,7 @@ skip_signoff_check = False # # Python isn't checked as flake8 performs these checks during build. line_length_ignore_list = re.compile( - r'\.(am|at|etc|in|m4|mk|patch|py)$|^debian/.*$') + r'\.(am|at|etc|in|m4|mk|patch|py|yml)$|^debian/.*$') # Don't enforce a requirement that leading whitespace be all spaces on # files that include these characters in their name, since these kinds
As far as I can tell they're used mostly for CI job definitions and these tend to result in long lines. Reported-at: https://mail.openvswitch.org/pipermail/ovs-dev/2023-June/405796.html Suggested-by: Aaron Conole <aconole@redhat.com> Signed-off-by: Dumitru Ceara <dceara@redhat.com> --- utilities/checkpatch.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)