Andrew / Allen, Could one of you file a YETUS ticket for a plugin that detects when the source tree is changed as a result of running the build? That sounds like a general error (for maven projects) that I'd like to see checked for even if my project doesn't use RAT.
On Mon, Oct 19, 2015 at 3:15 PM, Andrew Wang <andrew.w...@cloudera.com> wrote: > On Mon, Oct 19, 2015 at 12:39 PM, Allen Wittenauer <a...@altiscale.com> wrote: > >> >> On Oct 19, 2015, at 12:25 PM, Andrew Wang <andrew.w...@cloudera.com> >> wrote: >> >> >> > I do agree we should fix these usages of build/test/data, but I don't >> > follow the logic regarding running RAT after tests. The point of the >> > precommit RAT check is to avoid introducing things to the source tarball >> > that fail RAT, and test output is not part of the source tar ball. >> >> Yup. Which is why it’s bad that test data is getting written to >> the source area of the tree. >> >> > It's not >> > intended to detect issues if an RM incorrectly builds a release, the RM >> and >> > PMC always need to run RAT again when voting on a release. We don't run >> > precommit on releases, so this "RAT after tests" check wouldn't help >> detect >> > bad release tar balls. >> > >> > Point being, RAT is not supposed to be used to catch naughty tests that >> > operate outside target/, so let's not use it as such. These RAT errors, >> > although they helped us find some test issues, are unrelated to the >> release >> > process and thus spurious. >> >> I just had a very timely discussion with a dev here who is working >> on their first patch. They asked why sometimes (the trunk-version of) >> test-patch threw rat errors and sometimes it didn’t. After chatting, they >> were running it in dirty workspace mode after manually running unit tests. >> >> Sometimes it isn’t about people with positions. > > > For this first-time dev, their issue sounds like it should be addressed it > via better documentation or improved error messages. They could dirty their > workspace in any number of ways besides running tests and have the same > problem. The real fix is explaining to them the what and why of RAT, which > is not solved by running RAT last in precommit. -- Sean
