Frode Nordahl <[email protected]> writes: > On Thu, Sep 2, 2021 at 4:04 PM Aaron Conole <[email protected]> wrote: >> >> Hi Frode (et. al.), >> >> Frode Nordahl writes: >> >> > The checkpatch script will hang forever on line 282 of a otherwise >> > valid patch [0]. While the root of the issue is probably >> > somewhere between the regex itself and the Python re >> > implementation, this patch provides an early return to avoid this >> > specific issue. >> > >> > Also fix the docstring for the if_and_for_end_with_bracket_check >> > function. >> > >> > 0: >> > https://patchwork.ozlabs.org/project/ovn/patch/[email protected]/ >> > Signed-off-by: Frode Nordahl <frode.nordahl at canonical.com> >> > --- >> >> Sorry, I was on PTO when this patch came in and missed it. I don't >> agree with your proposed fix. It's a bit too conditional (and I was >> able to build a test-case that breaks it anyway). >> >> I see it's already been applied to OVN, but I plan to propose the >> following for OVS repository (and suggest a similar change to OVN). >> >> The change should be safe because we already validate that we are only >> in a for/if/while block, so the early bail to True should be okay. >> >> WDYT? > > No worries, I had to propose something as apparently I was clogging up > the CI, which is not nice :-) Your approach does look more generally > useful indeed and I would have no issue with having mine replaced by > it.
Cool! > However, it does end up with the opposite verdict on the line in > question, was that intentional? From my reading of the test, it checks > that when there is room for a bracket at the end of a line for a > conditional control block it should be there, does that not mean we > can put it on the next line when there is not room? I'm not sure what you mean. In Documentation/internals/contributing/coding-style.rst Section: Statements, I don't see anything about putting the brace on a separate line. So, I think such error case is correctly flagged. Maybe there could be an exception placed in the coding style - not sure. It's a different discussion, though. I'll propose this patch formally for OVS. I can also submit the patch for OVN, if you think that makes sense. _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
