On 8/3/22 13:44, Ilya Maximets wrote:
> On 8/3/22 12:39, Dumitru Ceara wrote:
>> Starting with flake8>=5.0.1 we got the following report:
>> tests/check_acl_log.py:94:80: E501 line too long (80 > 79 characters)
>>
>> Signed-off-by: Dumitru Ceara <[email protected]>
>> ---
>> tests/check_acl_log.py | 3 ++-
>> 1 file changed, 2 insertions(+), 1 deletion(-)
>>
>> diff --git a/tests/check_acl_log.py b/tests/check_acl_log.py
>> index 1dd9630c0..790846afe 100644
>> --- a/tests/check_acl_log.py
>> +++ b/tests/check_acl_log.py
>> @@ -91,7 +91,8 @@ def main():
>> try:
>> if parsed_log[key] != val:
>> print(
>> - f"Expected log {key}={val} but got
>> {key}={parsed_log[key]} \
>> + f"Expected log {key}={val} but got \
>> + {key}={parsed_log[key]} \
>> in:\n\t'{acl_log}"
>
> It's not a problem of this patch, but doesn't '\' introduce
> a bit number of unnecessary spaces in the string?
True, I had mechanically split it too.
>
> It should be just split into multiple strings, i.e.:
>
> f"Expected log {key}={val} but got "
> f"{key}={parsed_log[key]} in:\n\t'{acl_log}"
>
Fair point, I can prepare a v2 with this.
> (not sure what ' is for in that message)
Now that you mention it I think it's either unmatched or stray. I can
change it to:
f"{key}={parsed_log[key]} in:\n\t'{acl_log}'"
What do you think?
>
> Also, on a side note, f-strings are available only starting
> with python 3.6, while base requirement for OVN is 3.4.
I'm not sure what to do about this. We can also just give up on
f-strings for now. This is the only file where we use them as far as I
can tell. Or is it acceptable to bump the base requirement to 3.6?
Mark, what do you think?
> Also not really a problem of this patch.
>
> Best regards, Ilya Maximets.
>
Thanks for the review!
Dumitru
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
