Matt,
IMHO, you took the correct course of action.
Thank you for your diligence in resolving the issue in a timely fashion!
-daw-
On 2/8/2021 11:54 PM, Matthew Smith via lists.fd.io wrote:
On Mon, Feb 8, 2021 at 4:44 PM Matthew Smith via lists.fd.io
<http://lists.fd.io> <mgsmith=netgate....@lists.fd.io
<mailto:netgate....@lists.fd.io>> wrote:
Hi all,
I reviewed and submitted this change earlier today -
https://gerrit.fd.io/r/c/vpp/+/31162
<https://gerrit.fd.io/r/c/vpp/+/31162>. After it was merged, the
jenkins job 'vpp-merge-master-ubuntu1804-x86_64' failed because
two tests failed. The two failed tests seem related to the change
so I created a new change to revert the earlier one -
https://gerrit.fd.io/r/c/vpp/+/31178
<https://gerrit.fd.io/r/c/vpp/+/31178>. The checkstyle job failed
for the revert because the original patch removed some
pre-existing '/* INDENT-(ON|OFF) */' so the revert adds them back
in. It seems that checkstyle doesn't like that.
My question is.... should I try to fix the checkstyle errors or
just remove the -1 that jenkins set on the change and merge it as
is? I don't know if doing the latter will cause checkstyle to
continue complaining about INDENT-(ON|OFF) whenever someone
submits a new change. It's somewhat easy to fix those errors, but
then my "revert" would not be actually restoring the original
code. Maybe that doesn't matter?
Anyway, I'm trying to get the tests passing again while causing
the least possible amount of pain and/or confusion to others. If
anyone has a strong opinion on which option is better, I'd love to
hear it.
Thanks!
-Matt
I started to look into fixing the INDENT-(ON|OFF) that the checkstyle
job complained about with the reverted change (31178). When I ran
checkstyle.sh locally, it complained about many other formatting
issues besides INDENT-ON and INDENT-OFF. If I fixed them all, the
"reverted" code would have looked significantly different than the
original code, which seems wrong. So I just removed the jenkins -1
verify score and set it to +1 manually and merged it.
-Matt
-=-=-=-=-=-=-=-=-=-=-=-
Links: You receive all messages sent to this group.
View/Reply Online (#18710): https://lists.fd.io/g/vpp-dev/message/18710
Mute This Topic: https://lists.fd.io/mt/80491116/21656
Group Owner: vpp-dev+ow...@lists.fd.io
Unsubscribe: https://lists.fd.io/g/vpp-dev/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-
