IIRC Josh did some work towards this end back at the start of 2016. I haven't checked on the current state of it in a long time though:
https://builds.apache.org/job/PreCommit-ACCUMULO-Build/ On Mon, Jun 5, 2017 at 1:00 PM, Mike Drob <mad...@cloudera.com> wrote: > For what will be checked, maybe we ask nicely that somebody hook us in to > Apache Yetus and get a "standard" suite of checks for free, complete with > automated feedback. > > On Mon, Jun 5, 2017 at 12:54 PM, Dave Marion <dlmar...@comcast.net> wrote: > >> I have used Hadoop's documentation on this subject for submitting patches. >> I'm not suggesting that we go to this level of detail, but as a new >> contributor I know how to set up my IDE, what commands to run to create my >> patch, and I know the items that are going to be checked at the start. >> >> >> [1] https://wiki.apache.org/hadoop/HowToContribute >> >> [2] https://wiki.apache.org/hadoop/CodeReviewChecklist >> >> >> > >> > On June 5, 2017 at 1:19 PM Mike Miller <michaelpmil...@gmail.com> >> wrote: >> > >> > I could be wrong, but it sounds like there are two different >> > perspectives being discussed here and it may be helpful to try and >> > separate the two. On one hand there are discussions of guidelines for >> > reviewers (Dave's initial list, Keith's ideas) to follow and on the >> > other hand, suggestions for contributors, which Christopher's list >> > sounds more geared towards. Since everyone on this list has to wear >> > both hats, I think each different point of view could benefit from >> > some loose guidelines. >> > >> > For example, General Pull Request Guidelines for the Accumulo >> community: >> > When submitting a PR... please run these commands [...] before >> > submitting to ensure code adheres to checkstyle and passes findbugs, >> > etc >> > When reviewing a PR... ensure dialog portrays how strongly the >> > reviewer feels about the comment [Could = optional suggestion, Should >> > = would be helpful but not blocking, Must = required] >> > >> > On Mon, Jun 5, 2017 at 12:57 PM, Dave Marion <dlmar...@comcast.net> >> wrote: >> > >> > > > >> > > I think things can be improved when it comes to handling pull >> requests. The point of this thread was to try and come up with something to >> set expectations for the contributor. I figured the discussion would lead >> to the modification of the existing example or to a new example. >> Christopher provided a different example, but most of the feedback seemed >> to indicate that this was not warranted. I'm not sure what else I can say >> on the matter. If the majority thinks that its not a problem, then its not >> a problem. >> > > >> > > > > > >> > > > On June 5, 2017 at 12:39 PM Josh Elser < >> josh.el...@gmail.com> wrote: >> > > > >> > > > Perhaps this discussion would be better served if you >> gave some concrete >> > > > suggestions on how you think things can/should be >> improved. >> > > > >> > > > e.g. Mike's suggestion of using the >> maven-checkstyle-plugin earlier, why >> > > > not focus on that? Does this (still) work with the >> build? If so, how do >> > > > we get that run automagically via travis or jenkins? >> > > > >> > > > To me, it seems like you either wanted to throw some >> shade or you are >> > > > genuinely concerned about a problem that others are not >> (yet?) concerned >> > > > about. I doubt re-focusing contribution processes for >> efficiency would >> > > > be met with disapproval. >> > > > >> > > > On 6/5/17 12:32 PM, Dave Marion wrote: >> > > > >> > > > > > > > >> > > > > The main entrance to the community for new >> contributors is through pull requests. I have seen PR's approved in an >> inconsistent manner. My intent was to make known the expectations for new >> contributions so that newcomers don't get discouraged by the amount of >> feedback and/or changes requested while providing some guidelines to make >> it more consistent. It seems that there is not a desire to do this for >> various reasons. That's fine by me and I'm willing to drop the discussion >> here. >> > > > > >> > > > > > > > > > >> > > > > > On June 5, 2017 at 12:14 PM "Marc P." < >> marc.par...@gmail.com> wrote: >> > > > > > >> > > > > > Turner and Tubbs, >> > > > > > You both piqued my interest and I agree. >> There's something important in what both said regarding the discussion and >> importance of a particular change. Style changes most likely aren't deal >> breakers unless it is terribly confusing, but I would leave that up to the >> reviewer and developer to discuss. >> > > > > > >> > > > > > Dave, >> > > > > > I'm sure your intent is good and you goal >> isn't the handcuff reviewers. Is your concern over a stalemate on something >> such as a code style? Would a discussion not be the remedy for this? >> > > > > > >> > > > > > On Mon, Jun 5, 2017 at 12:07 PM, Keith >> Turner <ke...@deenlo.com mailto:ke...@deenlo.com > wrote: >> > > > > > >> > > > > > > > > > > > >> > > > > > > > > > > > > > >> > > > > > > > Sometimes I use review comments >> to just ask questions about things I >> > > > > > > > don't understand. Sometimes when >> looking at a code review, I have a >> > > > > > > > thought about the change that I >> know is a subjective opinion. In this >> > > > > > > > case I want to share my thought, >> in case they find it useful. >> > > > > > > > However, I don't care if a >> change is made or not. Sometimes I think a >> > > > > > > > change must be made. I try to >> communicate my intentions, but its >> > > > > > > > wordy, slow, and I don't think I >> always succeed. >> > > > > > > > >> > > > > > > > > > > > > > >> > > > > > > Given there are so many ways the >> comments on a review can be used, I >> > > > > > > think it can be difficult to quickly >> know the intentions of the >> > > > > > > reviewer. I liked review board's >> issues, I think they helped with >> > > > > > > this problem. A reviewer could make >> comments and issues. The issues >> > > > > > > made it clear what the reviewer >> thought must be done vs discussion. >> > > > > > > Issues made reviews more efficient by >> making the intentions clear AND >> > > > > > > separating important concerns from >> lots of discussion. >> > > > > > > >> > > > > > > When I submit a PR and it has lots of >> comments, towards the end I go >> > > > > > > back and look through all of the >> comments to make sure I didn't miss >> > > > > > > anything important. Its annoying to >> have to do this. Is there >> > > > > > > anything we could do in GH to >> replicate this and help separate the >> > > > > > > signal from the noise? >> > > > > > > >> > > > > > > On Mon, Jun 5, 2017 at 11:08 AM, Dave >> Marion <dlmar...@comcast.net mailto:dlmar...@comcast.net > wrote: >> > > > > > > >> > > > > > > > > > > > > > >> > > > > > > > I propose that we define a set >> of guidelines to use when reviewing pull requests. In doing so, >> contributors will be able to determine potential issues in their code >> possibly reducing the number of changes that occur before acceptance. >> Here's an example to start the discussion: >> > > > > > > > >> > > > > > > > Items a reviewer should look for: >> > > > > > > > >> > > > > > > > 1. Adherence to code >> formatting rules (link to formatting rules) >> > > > > > > > >> > > > > > > > 2. Unit tests required >> > > > > > > > >> > > > > > > > 3. Threading issues >> > > > > > > > >> > > > > > > > 4. Performance implications >> > > > > > > > >> > > > > > > > Items that should not block >> acceptance: >> > > > > > > > >> > > > > > > > 1. Stylistic changes that >> have no performance benefit >> > > > > > > > >> > > > > > > > 2. Addition of features >> outside the scope of the ticket (moving the goal post, discussion should >> lead to ticket creation) >> > > > > > > > >> > > > > > > > > > > > > > >> > > > > > > > >> > > > > > > >> > > > > > > > > > > > >> > > > > > > > > > >> > > > > > > > >> > > > > > >> > > > >> -- busbey
