FLUME-2706, FLUME-2919 and FLUME-2921 are the largest pending patches we currently have. But I think it's ok to resubmit those patches, it won't be too much work.
Also, are you planning to use google's style? https://github.com/checkstyle/checkstyle/blob/master/src/main/resources/google_checks.xml Are you going to use it as is? On Tue, Jun 28, 2016 at 10:48 PM, Mike Percy <[email protected]> wrote: > 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 > > > > > > > > > > > > > >
