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 <[email protected]>
>>>> Signed-off-by: Dumitru Ceara <[email protected]>
>>>> ---
>>>>  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
>>>
>>
> 

_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Reply via email to