On Mon, Feb 8, 2021 at 4:44 PM Matthew Smith via lists.fd.io <mgsmith= netgate....@lists.fd.io> wrote:
> Hi all, > > I reviewed and submitted this change earlier today - > 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. 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 (#18702): https://lists.fd.io/g/vpp-dev/message/18702 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] -=-=-=-=-=-=-=-=-=-=-=-
