My preference would be to get this in ASAP because I want to make code reviews less work. I'll make the patch (you are right, it is large) to make the existing code comply.
Personally I don't know of any huge patches that we are planning to merge for 1.7.0 so I don't really think it will be difficult to adapt existing patches. We're not changing every line across the code base, just the lines that don't comply. Do you know of any large patches that you think would have major conflicts? Maybe we should try to merge those first. Mike On Tue, Jun 28, 2016 at 12:34 PM, Lior Zeno <[email protected]> wrote: > I'm not sure that we should integrate that so close to the release, since > it won't only affect next commits, but it would require modifications to > our current code that violates the style. Usually this requires a great > deal of changes to many files in the project. I suggest fixing this issue > in 1.8.0 (FLUME-2937). > > On Tue, Jun 28, 2016 at 9:56 PM, Mike Percy <[email protected]> wrote: > > > Thanks everyone for your feedback. It sounds like have have consensus on > > this matter. In that case, I'm going to submit a patch to integrate > > checkstyle into the build. > > > > Mike > > > > On Mon, Jun 27, 2016 at 5:39 PM, Mike Percy <[email protected]> wrote: > > > > > On Mon, Jun 27, 2016 at 4:51 PM, Hari Shreedharan < > > [email protected] > > > > wrote: > > > > > >> I am not sure if a precommit hook will suffice, since we don't > actually > > >> run > > >> pre-commit builds. We will probably need to add it to the full build > so > > >> the > > >> developer can figure out the issues before even submitting the patch > for > > >> review. > > >> > > > > > > There is a way to get it to run as part of the Maven verify stage, > which > > > happens between the package and install phases [1]. So we wouldn't have > > to > > > run checkstyle when working on unit tests or running one of the tests, > > but > > > checkstyle would have to run to do a full build. It seems to take about > > 20 > > > seconds, which is hopefully tolerable but obviously not ideal when all > > you > > > want to do it build the code. > > > > > > I would rather do it as part of a pre-commit build hook, but we're not > > > there yet in terms of build stability or the Jenkins setup. I would > love > > to > > > see that Jenkins infrastructure revived and improved. Gerrit would be > > great > > > too. One step at a time. :) > > > > > > Mike > > > > > > [1] > > > > > > https://maven.apache.org/guides/introduction/introduction-to-the-lifecycle.html > > > > > > > > >
