On Thu, Mar 7, 2013 at 11:55 AM, David Medinets <[email protected]> wrote: > I thought it was a safe change because I made it to trunk. Sorry about > the disruption. Please revert. This issue is not worth spending any of > your time. And we can always run the script later to remove trailing > spaces. My concern is why there was a disruption? Could I have done > something better?
Experimenting with checkstyle rules that check on our current expectations about code formating would be a good start. This checkstyle plugin seems like a great thing and I think Accumulo should use it. We need to come up with a strategy for introducing this into 1.6. I suspect a lot of our code does not follow our current formatting expectations because we do not have checkstyle. So introducing it will probably necessitate reformatting code. Also, code formatting is something we decided as a community a while ago. IMO this changes to code formatting should be made as a community. Also, if we are going to change our code formatting rules we should decide on all of the changes we want to make, and make them at once. Making lots of little changes to the formatting rules over time seems very disruptive (complicates merges, user patches, and uncommitted changes in working area). One possible way to go about this is the following : * Send out email to dev list asking what code formatting rules changes people would like to make (possibly have a ticket) * After decision process settles, send another email to dev list proposing a date to reformat code in trunk and add checkstyle Keith > > On Thu, Mar 7, 2013 at 11:28 AM, Billie Rinaldi <[email protected]> wrote: >> Or we could apply the space changes to the 1.5 branch (not the pom change) >> ... >> >> Billie >> >> >> On Thu, Mar 7, 2013 at 11:24 AM, John Vines <[email protected]> wrote: >> >>> The only changes made to trunk since that were my merges, so it should be >>> pretty painless to roll it back, revert that change for now, and remerge. >>> >>> >>> On Thu, Mar 7, 2013 at 11:20 AM, Billie Rinaldi <[email protected]> wrote: >>> >>> > I don't mind if we roll it back until we stop doing so much merging. >>> > >>> > Billie >>> > >>> > >>> > On Thu, Mar 7, 2013 at 11:01 AM, John Vines <[email protected]> wrote: >>> > >>> > > Since it was introduced yesterday morning, every merge I've done had at >>> > > least 1 conflict file, usually multiple. And one to many merges per >>> file >>> > > (which is how I missed that one yesterday). >>> > > >>> > > >>> > > On Thu, Mar 7, 2013 at 10:57 AM, Keith Turner <[email protected]> >>> wrote: >>> > > >>> > > > On Thu, Mar 7, 2013 at 10:49 AM, John Vines <[email protected]> >>> wrote: >>> > > > > I've been getting unnecessary merge conflicts because of this >>> change. >>> > > At >>> > > > > the very least, I would like to see it reverted until we release >>> 1.5 >>> > > > >>> > > > Or maybe make the change after 1.5.1. Based on past experience, >>> there >>> > > > will likely be a good bit of merge activity from 1.5 to 1.6 until at >>> > > > least the first 1.5 bug fix release. >>> > > > >>> > > > Curious, how much extra time do this add to merging for you? I do >>> not >>> > > > have a good feeling for how well this will be handled automatically. >>> > > > Did it cause conflicts for most of the edits you made in 1.5? >>> > > > >>> > > > > >>> > > > > >>> > > > > On Thu, Mar 7, 2013 at 10:44 AM, Keith Turner <[email protected]> >>> > > wrote: >>> > > > > >>> > > > >> On Wed, Mar 6, 2013 at 10:23 AM, David Medinets >>> > > > >> <[email protected]> wrote: >>> > > > >> > I have a free day due to snowfall and while this is a fairly >>> silly >>> > > > >> > rule, writing a short script to rule all the java files through >>> > sed >>> > > > >> > should be fairly painless. As part of this change, I will >>> commit a >>> > > > >> > one-rule checkstyle.xml file which just runs this 'no spaces at >>> > end >>> > > of >>> > > > >> > line' rule. Over time, more rules can be added to that align >>> with >>> > > the >>> > > > >> > Accumulo community's style guidelines. >>> > > > >> > >>> > > > >> > Any objection? >>> > > > >> >>> > > > >> Whats the benefit of doing this? How will it impact merges from >>> 1.5 >>> > > > >> to 1.6? Should this be done for thrift generated code? >>> > > > >> >>> > > > >>> > > >>> > >>>
