On 6/25/25 12:27 PM, Changliang Wu wrote:
> New appctl 'lldp/neighbor' displays lldp neighbor information.
> Support json output with --format json --pretty
> Supoort dot1 and dot3 tlv
Hi. Thanks of the update! Please wait for Aaron or Eelco to review the
actual code before sending a new version, but see some overall style and
formatting comments below.
1. This patch set should be v7, AFAIU. The fact that it was converted to a
set doesn't make versions to reset. And list the version history with
the changes made here in the cover letter for v8.
2. Please, remove the long '-------------' lines from the command outputs,
OVS commands are not using this style normally and there is no point
in closely following someone else's format. Just put empty lines where
needed.
3. Don't use tabs in the command output. We removed all the tabs a few years
ago and shouldn't introduce new ones. Use spaces and printf syntax with
pre-defined length instead, e.g. %-20s. Tabs do not look good in the
output even here in the commit message as many lines are misaligned.
(Looks like there is one bonding command that still uses spaces, but there
is no reason to follow bad examples.)
4. While writing switch statements add an empty line between cases. See the
coding style document for examples:
Documentation/internals/contributing/coding-style.rst
5. Comments should generally be complete sentences that end with a dot.
Imported lldp code doesn't always follow that style, but it's better if
the new code follows the style guide.
6. Do not use real lldp packets mentioning real hardware in the tests.
Make some dummy packets with made-up manufacturer names and addresses.
7. Do not use jq in the tests. It's an extra dependency for the testsuite
and we don't have it installed in our CI. Hence the CI failure.
Best regards, Ilya Maximets.
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
