| Message ID | 458641657261297@vla1-d0568624e250.qloud-c.yandex.net |
|---|---|
| State | Superseded |
| Headers | show |
| Series | [ovs-dev] fix typo | expand |
| Context | Check | Description |
|---|---|---|
| ovsrobot/apply-robot | success | apply and check: success |
| ovsrobot/github-robot-_Build_and_Test | success | github build: passed |
| ovsrobot/github-robot-_ovn-kubernetes | success | github build: passed |
On 7/8/22 08:21, Zhukov Igor wrote: > I looked at https://github.com/ovn-org/ovn/commit/23e203a3f30bd57ef8d0def7ee2698e588782d0c > Found fixed typo: "webserer" => "webserver" > I decided to grep "webserer" and found another such word. Hi Igor, Thanks for the patch! > > From b1d7d16289ffa28f76c4a7803e369a5e287ffdc5 Mon Sep 17 00:00:00 2001 > From: Igor Zhukov <ivzhukov@sbercloud.ru> > Date: Fri, 8 Jul 2022 00:40:42 +0700 > Subject: [PATCH ovn] fix typo This should not be part of the commit message, it breaks the patch format and causes: $ git am /tmp/git-pwtngnq9u1/ovs-dev-fix-typo.patch Patch is empty. Removing these lines fixes the patch format. Also, I think this confuses 0-day robot (cc Aaron) which wrongfully reports a green CI [0] [1] run even though the patch never really got applied [2]: [0] https://mail.openvswitch.org/pipermail/ovs-build/2022-July/023384.html [1] https://github.com/ovsrobot/ovn/actions/runs/2634362231 [2] https://github.com/ovsrobot/ovn/tree/series_308632 For this specific case it's not an issue, the patch is obviously correct but we might want to avoid potential future false negatives for other patches. > > Signed-off-by: Igor Zhukov <ivzhukov@sbercloud.ru> > --- If you remove the 4 lines in the wrongfully formated patch then please also include my ack: Acked-by: Dumitru Ceara <dceara@redhat.com> Thanks! > tests/system-ovn.at | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/tests/system-ovn.at b/tests/system-ovn.at > index 1cabf1f31..4a8fdede8 100644 > --- a/tests/system-ovn.at > +++ b/tests/system-ovn.at > @@ -4505,7 +4505,7 @@ ovn-sbctl list service_monitor > OVS_WAIT_UNTIL([test 2 = `ovn-sbctl --bare --columns status find \ > service_monitor protocol=udp | sed '/^$/d' | grep offline | wc -l`]) > > -# Stop webserer in sw1-p1 > +# Stop webserver in sw1-p1 > pid_file=$(cat l7_pid_file) > NS_CHECK_EXEC([sw1-p1], [kill $(cat $pid_file)]) >
Dumitru Ceara <dceara@redhat.com> writes: > On 7/8/22 08:21, Zhukov Igor wrote: >> I looked at https://github.com/ovn-org/ovn/commit/23e203a3f30bd57ef8d0def7ee2698e588782d0c >> Found fixed typo: "webserer" => "webserver" >> I decided to grep "webserer" and found another such word. > > Hi Igor, > > Thanks for the patch! > >> >> From b1d7d16289ffa28f76c4a7803e369a5e287ffdc5 Mon Sep 17 00:00:00 2001 >> From: Igor Zhukov <ivzhukov@sbercloud.ru> >> Date: Fri, 8 Jul 2022 00:40:42 +0700 >> Subject: [PATCH ovn] fix typo > > This should not be part of the commit message, it breaks the patch > format and causes: > > $ git am /tmp/git-pwtngnq9u1/ovs-dev-fix-typo.patch > Patch is empty. > > Removing these lines fixes the patch format. > > Also, I think this confuses 0-day robot (cc Aaron) which wrongfully > reports a green CI [0] [1] run even though the patch never really got > applied [2]: > > [0] https://mail.openvswitch.org/pipermail/ovs-build/2022-July/023384.html > [1] https://github.com/ovsrobot/ovn/actions/runs/2634362231 > > [2] https://github.com/ovsrobot/ovn/tree/series_308632 > > For this specific case it's not an issue, the patch is obviously correct > but we might want to avoid potential future false negatives for other > patches. This is a difficult to detect situation. I will look at it. >> >> Signed-off-by: Igor Zhukov <ivzhukov@sbercloud.ru> >> --- > > If you remove the 4 lines in the wrongfully formated patch then please > also include my ack: > > Acked-by: Dumitru Ceara <dceara@redhat.com> > > Thanks! > >> tests/system-ovn.at | 2 +- >> 1 file changed, 1 insertion(+), 1 deletion(-) >> >> diff --git a/tests/system-ovn.at b/tests/system-ovn.at >> index 1cabf1f31..4a8fdede8 100644 >> --- a/tests/system-ovn.at >> +++ b/tests/system-ovn.at >> @@ -4505,7 +4505,7 @@ ovn-sbctl list service_monitor >> OVS_WAIT_UNTIL([test 2 = `ovn-sbctl --bare --columns status find \ >> service_monitor protocol=udp | sed '/^$/d' | grep offline | wc -l`]) >> >> -# Stop webserer in sw1-p1 >> +# Stop webserver in sw1-p1 >> pid_file=$(cat l7_pid_file) >> NS_CHECK_EXEC([sw1-p1], [kill $(cat $pid_file)]) >>
Thank you Aaron Conole! Sorry for so many attempts for such simple patch 😞. I think I know one reason why this issue might be: I sent this email from my home email: fsb4000@yandex.ru I also resent the patch from my work email, but I can't change my name and it is in russian. Жуков Игорь <ivzhukov at sbercloud.ru> Is it ok? or "Signed-off-by" shoud be exactly match?
On 7/8/22 17:32, Zhukov Igor wrote: > Thank you Aaron Conole! > Sorry for so many attempts for such simple patch 😞. > I think I know one reason why this issue might be: I sent this email from my home email: fsb4000@yandex.ru > > I also resent the patch from my work email, but I can't change my name and it is in russian. > > Жуков Игорь <ivzhukov at sbercloud.ru> > > Is it ok? or "Signed-off-by" shoud be exactly match? The "From:" header and the "Signed-off-by:" should generally match to not trigger checkpatch failures. You should try using 'git send-email' instead of your normal email client to send patches. It should save you from most of the issues. For example, it will add an extra 'From:' header for you in case the email you're sending from doesn't match with the patch author. Sending emails to yourself first to make sure that they are OK is also a good practice. Best regards, Ilya Maximets.
diff --git a/tests/system-ovn.at b/tests/system-ovn.at index 1cabf1f31..4a8fdede8 100644 --- a/tests/system-ovn.at +++ b/tests/system-ovn.at @@ -4505,7 +4505,7 @@ ovn-sbctl list service_monitor OVS_WAIT_UNTIL([test 2 = `ovn-sbctl --bare --columns status find \ service_monitor protocol=udp | sed '/^$/d' | grep offline | wc -l`]) -# Stop webserer in sw1-p1 +# Stop webserver in sw1-p1 pid_file=$(cat l7_pid_file) NS_CHECK_EXEC([sw1-p1], [kill $(cat $pid_file)])