Message ID | cover.1730917155.git.lorenzo.bianconi@redhat.com |
---|---|
Headers | show |
Series | Use check for ovn-nbctl/ovn-sbctl wherever possible. | expand |
Hi Lorenzo Thanks for the patches. For the series, there are (now) a few missing checks, probably due to the fact more patches got merged. We could also add specific checks for commands such as "ovn-nbctl create ..." which returns a uuid (i.e. add a check_uuid). Finally, we should add some additional verification in the utilities/checkpatch.py. I think that this should not prevent the patch to be merged as I can take care of that in a separate patch, to avoid additional rebases. So, it looks good to me. For the whole serie: Acked-by: Xavier Simonart <xsimonar@redhat.com> Thanks Xavier On Wed, Nov 6, 2024 at 7:22 PM Lorenzo Bianconi <lorenzo.bianconi@redhat.com> wrote: > Changes since v2: > - move test fixes in dedicated patches > > Reported-at: https://issues.redhat.com/browse/FDP-875 > > Lorenzo Bianconi (11): > test: ovn: Fix typos in unit-test > test: ovn-northd: Fix typos in unit-test > test: system-ovn: Fix typos in system-test > test: ovn-controller: Use check for ovn-nbctl wherever possible. > test: ovn: Use check for ovn-nbctl wherever possible. > test: ovn-northd: Use check for ovn-nbctl wherever possible. > test: ovn-performance: Use check for ovn-nbctl wherever possible. > test: ovn-ic: Use check for ovn-nbctl wherever possible. > test: perf-northd: Use check for ovn-nbctl wherever possible. > test: system-ovn: Use check for ovn-nbctl wherever possible. > test: Use check for ovn-sbctl wherever possible. > > tests/ovn-controller.at | 32 +- > tests/ovn-ic.at | 52 +- > tests/ovn-northd.at | 941 ++++++----- > tests/ovn-performance.at | 6 +- > tests/ovn.at | 3272 +++++++++++++++++++------------------- > tests/perf-northd.at | 4 +- > tests/system-ovn-kmod.at | 306 ++-- > tests/system-ovn.at | 1890 +++++++++++----------- > 8 files changed, 3250 insertions(+), 3253 deletions(-) > > -- > 2.47.0 > >
> Hi Lorenzo > > Thanks for the patches. > > For the series, there are (now) a few missing checks, probably due to the > fact more patches got merged. Hi Xavier, thx for looking at the series. Do you want me to add missing 'check' and post v4 or are you taking care of it? Regards, Lorenzo > We could also add specific checks for commands such as "ovn-nbctl create > ..." which returns a uuid (i.e. add a check_uuid). > Finally, we should add some additional verification in the > utilities/checkpatch.py. > > I think that this should not prevent the patch to be merged as I can take > care of that in a separate patch, to avoid additional rebases. > > So, it looks good to me. > For the whole serie: > Acked-by: Xavier Simonart <xsimonar@redhat.com> > > Thanks > Xavier > > On Wed, Nov 6, 2024 at 7:22 PM Lorenzo Bianconi <lorenzo.bianconi@redhat.com> > wrote: > > > Changes since v2: > > - move test fixes in dedicated patches > > > > Reported-at: https://issues.redhat.com/browse/FDP-875 > > > > Lorenzo Bianconi (11): > > test: ovn: Fix typos in unit-test > > test: ovn-northd: Fix typos in unit-test > > test: system-ovn: Fix typos in system-test > > test: ovn-controller: Use check for ovn-nbctl wherever possible. > > test: ovn: Use check for ovn-nbctl wherever possible. > > test: ovn-northd: Use check for ovn-nbctl wherever possible. > > test: ovn-performance: Use check for ovn-nbctl wherever possible. > > test: ovn-ic: Use check for ovn-nbctl wherever possible. > > test: perf-northd: Use check for ovn-nbctl wherever possible. > > test: system-ovn: Use check for ovn-nbctl wherever possible. > > test: Use check for ovn-sbctl wherever possible. > > > > tests/ovn-controller.at | 32 +- > > tests/ovn-ic.at | 52 +- > > tests/ovn-northd.at | 941 ++++++----- > > tests/ovn-performance.at | 6 +- > > tests/ovn.at | 3272 +++++++++++++++++++------------------- > > tests/perf-northd.at | 4 +- > > tests/system-ovn-kmod.at | 306 ++-- > > tests/system-ovn.at | 1890 +++++++++++----------- > > 8 files changed, 3250 insertions(+), 3253 deletions(-) > > > > -- > > 2.47.0 > > > >
Hi Lorenzo On Tue, Nov 26, 2024 at 6:08 PM Lorenzo Bianconi < lorenzo.bianconi@redhat.com> wrote: > > Hi Lorenzo > > > > Thanks for the patches. > > > > For the series, there are (now) a few missing checks, probably due to the > > fact more patches got merged. > > Hi Xavier, > > thx for looking at the series. Do you want me to add missing 'check' and > post > v4 or are you taking care of it? > I just realized that there are in fact many missing checks: all ovn-nbctl/ovn-sbctl prepended with spaces/tabs (e.g. hitting ^\s*ovn-nbctl) However, if ok for mergers, I can do it in further patches as I can use an updated checkpatch.py to detect those. Thanks Xavier > > Regards, > Lorenzo > > > We could also add specific checks for commands such as "ovn-nbctl create > > ..." which returns a uuid (i.e. add a check_uuid). > > Finally, we should add some additional verification in the > > utilities/checkpatch.py. > > > > I think that this should not prevent the patch to be merged as I can take > > care of that in a separate patch, to avoid additional rebases. > > > > So, it looks good to me. > > For the whole serie: > > Acked-by: Xavier Simonart <xsimonar@redhat.com> > > > > Thanks > > Xavier > > > > On Wed, Nov 6, 2024 at 7:22 PM Lorenzo Bianconi < > lorenzo.bianconi@redhat.com> > > wrote: > > > > > Changes since v2: > > > - move test fixes in dedicated patches > > > > > > Reported-at: https://issues.redhat.com/browse/FDP-875 > > > > > > Lorenzo Bianconi (11): > > > test: ovn: Fix typos in unit-test > > > test: ovn-northd: Fix typos in unit-test > > > test: system-ovn: Fix typos in system-test > > > test: ovn-controller: Use check for ovn-nbctl wherever possible. > > > test: ovn: Use check for ovn-nbctl wherever possible. > > > test: ovn-northd: Use check for ovn-nbctl wherever possible. > > > test: ovn-performance: Use check for ovn-nbctl wherever possible. > > > test: ovn-ic: Use check for ovn-nbctl wherever possible. > > > test: perf-northd: Use check for ovn-nbctl wherever possible. > > > test: system-ovn: Use check for ovn-nbctl wherever possible. > > > test: Use check for ovn-sbctl wherever possible. > > > > > > tests/ovn-controller.at | 32 +- > > > tests/ovn-ic.at | 52 +- > > > tests/ovn-northd.at | 941 ++++++----- > > > tests/ovn-performance.at | 6 +- > > > tests/ovn.at | 3272 > +++++++++++++++++++------------------- > > > tests/perf-northd.at | 4 +- > > > tests/system-ovn-kmod.at | 306 ++-- > > > tests/system-ovn.at | 1890 +++++++++++----------- > > > 8 files changed, 3250 insertions(+), 3253 deletions(-) > > > > > > -- > > > 2.47.0 > > > > > > >
Hi Lorenzo, Xavier, On 11/27/24 12:40 PM, Xavier Simonart wrote: > Hi Lorenzo > > On Tue, Nov 26, 2024 at 6:08 PM Lorenzo Bianconi < > lorenzo.bianconi@redhat.com> wrote: > >>> Hi Lorenzo >>> >>> Thanks for the patches. >>> >>> For the series, there are (now) a few missing checks, probably due to the >>> fact more patches got merged. >> >> Hi Xavier, >> >> thx for looking at the series. Do you want me to add missing 'check' and >> post >> v4 or are you taking care of it? >> > I just realized that there are in fact many missing checks: all > ovn-nbctl/ovn-sbctl prepended with spaces/tabs > (e.g. hitting ^\s*ovn-nbctl) > However, if ok for mergers, I can do it in further patches as I can use an > updated checkpatch.py to detect those. I applied the series to main. I backported the first 3 test fixes to 24.09 and 24.03. I'm looking forward to the follow up and updated checkpatch script. Thanks, DUmitru > Thanks > Xavier > >> >> Regards, >> Lorenzo >> >>> We could also add specific checks for commands such as "ovn-nbctl create >>> ..." which returns a uuid (i.e. add a check_uuid). >>> Finally, we should add some additional verification in the >>> utilities/checkpatch.py. >>> >>> I think that this should not prevent the patch to be merged as I can take >>> care of that in a separate patch, to avoid additional rebases. >>> >>> So, it looks good to me. >>> For the whole serie: >>> Acked-by: Xavier Simonart <xsimonar@redhat.com> >>> >>> Thanks >>> Xavier >>> >>> On Wed, Nov 6, 2024 at 7:22 PM Lorenzo Bianconi < >> lorenzo.bianconi@redhat.com> >>> wrote: >>> >>>> Changes since v2: >>>> - move test fixes in dedicated patches >>>> >>>> Reported-at: https://issues.redhat.com/browse/FDP-875 >>>> >>>> Lorenzo Bianconi (11): >>>> test: ovn: Fix typos in unit-test >>>> test: ovn-northd: Fix typos in unit-test >>>> test: system-ovn: Fix typos in system-test >>>> test: ovn-controller: Use check for ovn-nbctl wherever possible. >>>> test: ovn: Use check for ovn-nbctl wherever possible. >>>> test: ovn-northd: Use check for ovn-nbctl wherever possible. >>>> test: ovn-performance: Use check for ovn-nbctl wherever possible. >>>> test: ovn-ic: Use check for ovn-nbctl wherever possible. >>>> test: perf-northd: Use check for ovn-nbctl wherever possible. >>>> test: system-ovn: Use check for ovn-nbctl wherever possible. >>>> test: Use check for ovn-sbctl wherever possible. >>>> >>>> tests/ovn-controller.at | 32 +- >>>> tests/ovn-ic.at | 52 +- >>>> tests/ovn-northd.at | 941 ++++++----- >>>> tests/ovn-performance.at | 6 +- >>>> tests/ovn.at | 3272 >> +++++++++++++++++++------------------- >>>> tests/perf-northd.at | 4 +- >>>> tests/system-ovn-kmod.at | 306 ++-- >>>> tests/system-ovn.at | 1890 +++++++++++----------- >>>> 8 files changed, 3250 insertions(+), 3253 deletions(-) >>>> >>>> -- >>>> 2.47.0 >>>> >>>> >> > _______________________________________________ > dev mailing list > dev@openvswitch.org > https://mail.openvswitch.org/mailman/listinfo/ovs-dev