You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@nifi.apache.org by Joe Witt <jo...@gmail.com> on 2015/03/17 19:02:11 UTC
lack of consistent formatting - how do others clean this up?
Folks,
It was brought up to me by a contributor the other day that our lack
of consistent formatting makes reviewing true changes difficult. I
see it myself quite often. There are difference between formatting in
Eclipse/Netbeans/manual VI/etc..
I am assuming there are good plugins to just automate this sort of
stuff. What do you all think as a good way to go about this?
Thanks
Joe
Re: lack of consistent formatting - how do others clean this up?
Posted by Joe Witt <jo...@gmail.com>.
Ok. So on branch NIFI-271 have added checkstyle (with the exact rules
accumulo uses). As you could predict the build does not work at this
time because of checkstyle failures in a ton of places. I am happy to
go through the entire codebase and make those changes.
But, I'll hold here for two things:
1) Give folks a chance to take a look at those checkstyle rules.
They're on branch NIFI-250 in nifi-parent/pom.xml. Now is a good time
if you do indeed have a strong opinion to influence the near-term
outcome. I think the discussion preceding this suggests we're all
open to the fact that formatting is a holy war none of us want. But
what we wall want is consistency. So this sets a stake in the ground.
We can iterate from there, albeit slowly, over time. Please if you
are likely to care about this take a moment to checkout the branch and
using your favorite IDE/formatter of choice make sure things seem ok.
I use netbeans <hold the jokes please> and found these rules to be
perfectly fine without having to tweak much. Specifically for those
of you who use Eclipse or IntelliJ or vi please give it a look.
2) There is a very major branch NIFI-250 outstanding for which it
would really ruin their party if i made all of these changes now. So
please advise when that branch can be folded into develop as well as
when it has been. If anyone else has outstanding branches for which
they would like to see this initiative held up please identify them so
we can minimize the challenge faced later when attempting to merge.
Once we're past 1 & 2 here I'll do the following:
1) Rebase/Merge (whatever I can get away with) NIFI-271 against
whatever the lastest development line has.
2) Build. Resolve style failures of noted module <repeat until happy>
3) Request a prompt RTC and merge into develop
Finally, I do not know whether this resolves all of the formatting
cases which can cause merges to require more effort than intuitively
necessary. But this should at least get us closer to consistent.
We'll need to work quickly on that part because the longer it takes
the higher the probably of disruption to folks who might want to do a
contribution.
We're seeing more and more contributions and that will only continue
if we make it easy for people to contribute quality code. So this is
a really important step.
Thanks!
Joe
On Sat, Mar 21, 2015 at 11:18 AM, Joe Witt <jo...@gmail.com> wrote:
> Folks,
>
> Benson put in a ticket a while back:
> https://issues.apache.org/jira/browse/NIFI-271 to make a DRY
> nifi-parent we can extend from in the main nifi line and the
> nifi-nar-plugin.
>
> Proposal:
> 1) Do what Benson said.
> 2) In that nifi-parent ensure checkstyle is always run and thus
> consistent across any nifi item. Fail the build if any violations.
> 3) In that nifi-parent ensure check-licenses is always run and if any
> fails - fail the build.
>
> Commentary:
> - This is not as forgiving as Sean suggested but it also does not
> preclude us from doing the QC bot to check higher order items.
> - This is more in-line with Adam's suggestion but gives the
> contributor direct feedback on what is wrong that they can resolve on
> their own without us rejecting their PR. This I am guessing was
> Adam's real intent anyway.
> - I will go through an make sure all existing code is in-line with the
> checkstyle form that we will create. That will require very loud
> music and good drinks but whatever - about as much fun as it was
> getting all the licensing squared away.
>
> I noticed that accumulo has this nicely integrated into their build so
> that gives a great example to follow.
>
> Thanks
> Joe
>
> On Tue, Mar 17, 2015 at 10:44 PM, Joe Witt <jo...@gmail.com> wrote:
>> Sean:
>>
>> Nope we're still pretty basic.
>>
>> Thanks
>> Joe
>>
>> On Tue, Mar 17, 2015 at 10:41 PM, Sean Busbey <bu...@cloudera.com> wrote:
>>> On Tue, Mar 17, 2015 at 9:32 PM, Joe Witt <jo...@gmail.com> wrote:
>>>
>>>>
>>>>
>>>> On the thread itself: Anyone interested in pushing forward the
>>>> model/changes to get the formatting process smoothed out please do so.
>>>>
>>>>
>>> Do y'all have a QA bot yet? I'm looking to coalesce the pre-commit testing
>>> of Hadoop and HBase in the next ~2-4 weeks. Having a third unrelated
>>> project to throw against that would help me ensure I have something
>>> reusable that can spread across ASF projects.
>>>
>>> We haven't determined yet where the shared pre-commit checker will live,
>>> but we don't seem too opinionated yet so it's unlikely we'll need lots of
>>> changes.
>>>
>>> --
>>> Sean
Re: lack of consistent formatting - how do others clean this up?
Posted by Joe Witt <jo...@gmail.com>.
The contributor guide is a necessary document for sure.
I plan to do the NIFI-271 work soon (hopefully tonight or this
weekend). It will be annoying for folks no matter when we do it. So,
my apologies in advance for this necessary evil.
Thanks
Joe
On Mon, Apr 6, 2015 at 1:56 PM, Ryan Blue <bl...@cloudera.com> wrote:
> On 04/05/2015 07:37 PM, Joe Witt wrote:
>>
>> Ok so NIFI-250 has been merged. Can I go ahead and do the great
>> reformatting?
>>
>> Thanks
>> Joe
>
>
> +1
>
> Have we added style guidelines in the contributor guide?
>
>
> rb
>
>
> --
> Ryan Blue
> Software Engineer
> Cloudera, Inc.
Re: lack of consistent formatting - how do others clean this up?
Posted by Ryan Blue <bl...@cloudera.com>.
On 04/05/2015 07:37 PM, Joe Witt wrote:
> Ok so NIFI-250 has been merged. Can I go ahead and do the great reformatting?
>
> Thanks
> Joe
+1
Have we added style guidelines in the contributor guide?
rb
--
Ryan Blue
Software Engineer
Cloudera, Inc.
Re: lack of consistent formatting - how do others clean this up?
Posted by Matt Gilman <ma...@gmail.com>.
To my knowledge there's nothing else that should be an issue.
Matt
Sent from my iPhone
> On Apr 5, 2015, at 10:37 PM, Joe Witt <jo...@gmail.com> wrote:
>
> Ok so NIFI-250 has been merged. Can I go ahead and do the great reformatting?
>
> Thanks
> Joe
>
>> On Mon, Mar 30, 2015 at 3:30 PM, Dan Bress <db...@onyxconsults.com> wrote:
>> Joe,
>> I don't have anything big I am working on presently, except NIFI-463 that is done and waiting to be merged. No need to hold on NIFI-271 on my behalf.
>>
>> Dan Bress
>> Software Engineer
>> ONYX Consulting Services
>>
>> ________________________________________
>> From: Joe Witt <jo...@gmail.com>
>> Sent: Monday, March 30, 2015 9:06 AM
>> To: dev@nifi.incubator.apache.org
>> Subject: Re: lack of consistent formatting - how do others clean this up?
>>
>> Hello All,
>>
>> Wanted to ping and find out how close we are to being able to do the
>> great reformat?
>>
>> I had the incorrect branch for folks to review if they wanted to mess
>> with the checkstyle rules. It should have been NIFI-271.
>>
>> We're holding for NIFI-250. Just pinging because the longer we wait
>> the more disruptive it is to PRs that folks are working. I know Dan B
>> and Toivo are both working larger efforts so don't want to create too
>> much pain for them when merging.
>>
>> Thanks
>> Joe
>>
>>> On Sat, Mar 21, 2015 at 1:55 PM, Tony Kurc <tr...@gmail.com> wrote:
>>> Joe,
>>> I like your proposal.
>>>
>>>> On Sat, Mar 21, 2015 at 11:18 AM, Joe Witt <jo...@gmail.com> wrote:
>>>>
>>>> Folks,
>>>>
>>>> Benson put in a ticket a while back:
>>>> https://issues.apache.org/jira/browse/NIFI-271 to make a DRY
>>>> nifi-parent we can extend from in the main nifi line and the
>>>> nifi-nar-plugin.
>>>>
>>>> Proposal:
>>>> 1) Do what Benson said.
>>>> 2) In that nifi-parent ensure checkstyle is always run and thus
>>>> consistent across any nifi item. Fail the build if any violations.
>>>> 3) In that nifi-parent ensure check-licenses is always run and if any
>>>> fails - fail the build.
>>>>
>>>> Commentary:
>>>> - This is not as forgiving as Sean suggested but it also does not
>>>> preclude us from doing the QC bot to check higher order items.
>>>> - This is more in-line with Adam's suggestion but gives the
>>>> contributor direct feedback on what is wrong that they can resolve on
>>>> their own without us rejecting their PR. This I am guessing was
>>>> Adam's real intent anyway.
>>>> - I will go through an make sure all existing code is in-line with the
>>>> checkstyle form that we will create. That will require very loud
>>>> music and good drinks but whatever - about as much fun as it was
>>>> getting all the licensing squared away.
>>>>
>>>> I noticed that accumulo has this nicely integrated into their build so
>>>> that gives a great example to follow.
>>>>
>>>> Thanks
>>>> Joe
>>>>
>>>>> On Tue, Mar 17, 2015 at 10:44 PM, Joe Witt <jo...@gmail.com> wrote:
>>>>> Sean:
>>>>>
>>>>> Nope we're still pretty basic.
>>>>>
>>>>> Thanks
>>>>> Joe
>>>>>
>>>>> On Tue, Mar 17, 2015 at 10:41 PM, Sean Busbey <bu...@cloudera.com>
>>>> wrote:
>>>>>> On Tue, Mar 17, 2015 at 9:32 PM, Joe Witt <jo...@gmail.com> wrote:
>>>>>>
>>>>>>>
>>>>>>>
>>>>>>> On the thread itself: Anyone interested in pushing forward the
>>>>>>> model/changes to get the formatting process smoothed out please do so.
>>>>>>>
>>>>>>>
>>>>>> Do y'all have a QA bot yet? I'm looking to coalesce the pre-commit
>>>> testing
>>>>>> of Hadoop and HBase in the next ~2-4 weeks. Having a third unrelated
>>>>>> project to throw against that would help me ensure I have something
>>>>>> reusable that can spread across ASF projects.
>>>>>>
>>>>>> We haven't determined yet where the shared pre-commit checker will live,
>>>>>> but we don't seem too opinionated yet so it's unlikely we'll need lots
>>>> of
>>>>>> changes.
>>>>>>
>>>>>> --
>>>>>> Sean
>>>>
Re: lack of consistent formatting - how do others clean this up?
Posted by Joe Witt <jo...@gmail.com>.
Ok so NIFI-250 has been merged. Can I go ahead and do the great reformatting?
Thanks
Joe
On Mon, Mar 30, 2015 at 3:30 PM, Dan Bress <db...@onyxconsults.com> wrote:
> Joe,
> I don't have anything big I am working on presently, except NIFI-463 that is done and waiting to be merged. No need to hold on NIFI-271 on my behalf.
>
> Dan Bress
> Software Engineer
> ONYX Consulting Services
>
> ________________________________________
> From: Joe Witt <jo...@gmail.com>
> Sent: Monday, March 30, 2015 9:06 AM
> To: dev@nifi.incubator.apache.org
> Subject: Re: lack of consistent formatting - how do others clean this up?
>
> Hello All,
>
> Wanted to ping and find out how close we are to being able to do the
> great reformat?
>
> I had the incorrect branch for folks to review if they wanted to mess
> with the checkstyle rules. It should have been NIFI-271.
>
> We're holding for NIFI-250. Just pinging because the longer we wait
> the more disruptive it is to PRs that folks are working. I know Dan B
> and Toivo are both working larger efforts so don't want to create too
> much pain for them when merging.
>
> Thanks
> Joe
>
> On Sat, Mar 21, 2015 at 1:55 PM, Tony Kurc <tr...@gmail.com> wrote:
>> Joe,
>> I like your proposal.
>>
>> On Sat, Mar 21, 2015 at 11:18 AM, Joe Witt <jo...@gmail.com> wrote:
>>
>>> Folks,
>>>
>>> Benson put in a ticket a while back:
>>> https://issues.apache.org/jira/browse/NIFI-271 to make a DRY
>>> nifi-parent we can extend from in the main nifi line and the
>>> nifi-nar-plugin.
>>>
>>> Proposal:
>>> 1) Do what Benson said.
>>> 2) In that nifi-parent ensure checkstyle is always run and thus
>>> consistent across any nifi item. Fail the build if any violations.
>>> 3) In that nifi-parent ensure check-licenses is always run and if any
>>> fails - fail the build.
>>>
>>> Commentary:
>>> - This is not as forgiving as Sean suggested but it also does not
>>> preclude us from doing the QC bot to check higher order items.
>>> - This is more in-line with Adam's suggestion but gives the
>>> contributor direct feedback on what is wrong that they can resolve on
>>> their own without us rejecting their PR. This I am guessing was
>>> Adam's real intent anyway.
>>> - I will go through an make sure all existing code is in-line with the
>>> checkstyle form that we will create. That will require very loud
>>> music and good drinks but whatever - about as much fun as it was
>>> getting all the licensing squared away.
>>>
>>> I noticed that accumulo has this nicely integrated into their build so
>>> that gives a great example to follow.
>>>
>>> Thanks
>>> Joe
>>>
>>> On Tue, Mar 17, 2015 at 10:44 PM, Joe Witt <jo...@gmail.com> wrote:
>>> > Sean:
>>> >
>>> > Nope we're still pretty basic.
>>> >
>>> > Thanks
>>> > Joe
>>> >
>>> > On Tue, Mar 17, 2015 at 10:41 PM, Sean Busbey <bu...@cloudera.com>
>>> wrote:
>>> >> On Tue, Mar 17, 2015 at 9:32 PM, Joe Witt <jo...@gmail.com> wrote:
>>> >>
>>> >>>
>>> >>>
>>> >>> On the thread itself: Anyone interested in pushing forward the
>>> >>> model/changes to get the formatting process smoothed out please do so.
>>> >>>
>>> >>>
>>> >> Do y'all have a QA bot yet? I'm looking to coalesce the pre-commit
>>> testing
>>> >> of Hadoop and HBase in the next ~2-4 weeks. Having a third unrelated
>>> >> project to throw against that would help me ensure I have something
>>> >> reusable that can spread across ASF projects.
>>> >>
>>> >> We haven't determined yet where the shared pre-commit checker will live,
>>> >> but we don't seem too opinionated yet so it's unlikely we'll need lots
>>> of
>>> >> changes.
>>> >>
>>> >> --
>>> >> Sean
>>>
Re: lack of consistent formatting - how do others clean this up?
Posted by Dan Bress <db...@onyxconsults.com>.
Joe,
I don't have anything big I am working on presently, except NIFI-463 that is done and waiting to be merged. No need to hold on NIFI-271 on my behalf.
Dan Bress
Software Engineer
ONYX Consulting Services
________________________________________
From: Joe Witt <jo...@gmail.com>
Sent: Monday, March 30, 2015 9:06 AM
To: dev@nifi.incubator.apache.org
Subject: Re: lack of consistent formatting - how do others clean this up?
Hello All,
Wanted to ping and find out how close we are to being able to do the
great reformat?
I had the incorrect branch for folks to review if they wanted to mess
with the checkstyle rules. It should have been NIFI-271.
We're holding for NIFI-250. Just pinging because the longer we wait
the more disruptive it is to PRs that folks are working. I know Dan B
and Toivo are both working larger efforts so don't want to create too
much pain for them when merging.
Thanks
Joe
On Sat, Mar 21, 2015 at 1:55 PM, Tony Kurc <tr...@gmail.com> wrote:
> Joe,
> I like your proposal.
>
> On Sat, Mar 21, 2015 at 11:18 AM, Joe Witt <jo...@gmail.com> wrote:
>
>> Folks,
>>
>> Benson put in a ticket a while back:
>> https://issues.apache.org/jira/browse/NIFI-271 to make a DRY
>> nifi-parent we can extend from in the main nifi line and the
>> nifi-nar-plugin.
>>
>> Proposal:
>> 1) Do what Benson said.
>> 2) In that nifi-parent ensure checkstyle is always run and thus
>> consistent across any nifi item. Fail the build if any violations.
>> 3) In that nifi-parent ensure check-licenses is always run and if any
>> fails - fail the build.
>>
>> Commentary:
>> - This is not as forgiving as Sean suggested but it also does not
>> preclude us from doing the QC bot to check higher order items.
>> - This is more in-line with Adam's suggestion but gives the
>> contributor direct feedback on what is wrong that they can resolve on
>> their own without us rejecting their PR. This I am guessing was
>> Adam's real intent anyway.
>> - I will go through an make sure all existing code is in-line with the
>> checkstyle form that we will create. That will require very loud
>> music and good drinks but whatever - about as much fun as it was
>> getting all the licensing squared away.
>>
>> I noticed that accumulo has this nicely integrated into their build so
>> that gives a great example to follow.
>>
>> Thanks
>> Joe
>>
>> On Tue, Mar 17, 2015 at 10:44 PM, Joe Witt <jo...@gmail.com> wrote:
>> > Sean:
>> >
>> > Nope we're still pretty basic.
>> >
>> > Thanks
>> > Joe
>> >
>> > On Tue, Mar 17, 2015 at 10:41 PM, Sean Busbey <bu...@cloudera.com>
>> wrote:
>> >> On Tue, Mar 17, 2015 at 9:32 PM, Joe Witt <jo...@gmail.com> wrote:
>> >>
>> >>>
>> >>>
>> >>> On the thread itself: Anyone interested in pushing forward the
>> >>> model/changes to get the formatting process smoothed out please do so.
>> >>>
>> >>>
>> >> Do y'all have a QA bot yet? I'm looking to coalesce the pre-commit
>> testing
>> >> of Hadoop and HBase in the next ~2-4 weeks. Having a third unrelated
>> >> project to throw against that would help me ensure I have something
>> >> reusable that can spread across ASF projects.
>> >>
>> >> We haven't determined yet where the shared pre-commit checker will live,
>> >> but we don't seem too opinionated yet so it's unlikely we'll need lots
>> of
>> >> changes.
>> >>
>> >> --
>> >> Sean
>>
Re: lack of consistent formatting - how do others clean this up?
Posted by Joe Witt <jo...@gmail.com>.
Hello All,
Wanted to ping and find out how close we are to being able to do the
great reformat?
I had the incorrect branch for folks to review if they wanted to mess
with the checkstyle rules. It should have been NIFI-271.
We're holding for NIFI-250. Just pinging because the longer we wait
the more disruptive it is to PRs that folks are working. I know Dan B
and Toivo are both working larger efforts so don't want to create too
much pain for them when merging.
Thanks
Joe
On Sat, Mar 21, 2015 at 1:55 PM, Tony Kurc <tr...@gmail.com> wrote:
> Joe,
> I like your proposal.
>
> On Sat, Mar 21, 2015 at 11:18 AM, Joe Witt <jo...@gmail.com> wrote:
>
>> Folks,
>>
>> Benson put in a ticket a while back:
>> https://issues.apache.org/jira/browse/NIFI-271 to make a DRY
>> nifi-parent we can extend from in the main nifi line and the
>> nifi-nar-plugin.
>>
>> Proposal:
>> 1) Do what Benson said.
>> 2) In that nifi-parent ensure checkstyle is always run and thus
>> consistent across any nifi item. Fail the build if any violations.
>> 3) In that nifi-parent ensure check-licenses is always run and if any
>> fails - fail the build.
>>
>> Commentary:
>> - This is not as forgiving as Sean suggested but it also does not
>> preclude us from doing the QC bot to check higher order items.
>> - This is more in-line with Adam's suggestion but gives the
>> contributor direct feedback on what is wrong that they can resolve on
>> their own without us rejecting their PR. This I am guessing was
>> Adam's real intent anyway.
>> - I will go through an make sure all existing code is in-line with the
>> checkstyle form that we will create. That will require very loud
>> music and good drinks but whatever - about as much fun as it was
>> getting all the licensing squared away.
>>
>> I noticed that accumulo has this nicely integrated into their build so
>> that gives a great example to follow.
>>
>> Thanks
>> Joe
>>
>> On Tue, Mar 17, 2015 at 10:44 PM, Joe Witt <jo...@gmail.com> wrote:
>> > Sean:
>> >
>> > Nope we're still pretty basic.
>> >
>> > Thanks
>> > Joe
>> >
>> > On Tue, Mar 17, 2015 at 10:41 PM, Sean Busbey <bu...@cloudera.com>
>> wrote:
>> >> On Tue, Mar 17, 2015 at 9:32 PM, Joe Witt <jo...@gmail.com> wrote:
>> >>
>> >>>
>> >>>
>> >>> On the thread itself: Anyone interested in pushing forward the
>> >>> model/changes to get the formatting process smoothed out please do so.
>> >>>
>> >>>
>> >> Do y'all have a QA bot yet? I'm looking to coalesce the pre-commit
>> testing
>> >> of Hadoop and HBase in the next ~2-4 weeks. Having a third unrelated
>> >> project to throw against that would help me ensure I have something
>> >> reusable that can spread across ASF projects.
>> >>
>> >> We haven't determined yet where the shared pre-commit checker will live,
>> >> but we don't seem too opinionated yet so it's unlikely we'll need lots
>> of
>> >> changes.
>> >>
>> >> --
>> >> Sean
>>
Re: lack of consistent formatting - how do others clean this up?
Posted by Tony Kurc <tr...@gmail.com>.
Joe,
I like your proposal.
On Sat, Mar 21, 2015 at 11:18 AM, Joe Witt <jo...@gmail.com> wrote:
> Folks,
>
> Benson put in a ticket a while back:
> https://issues.apache.org/jira/browse/NIFI-271 to make a DRY
> nifi-parent we can extend from in the main nifi line and the
> nifi-nar-plugin.
>
> Proposal:
> 1) Do what Benson said.
> 2) In that nifi-parent ensure checkstyle is always run and thus
> consistent across any nifi item. Fail the build if any violations.
> 3) In that nifi-parent ensure check-licenses is always run and if any
> fails - fail the build.
>
> Commentary:
> - This is not as forgiving as Sean suggested but it also does not
> preclude us from doing the QC bot to check higher order items.
> - This is more in-line with Adam's suggestion but gives the
> contributor direct feedback on what is wrong that they can resolve on
> their own without us rejecting their PR. This I am guessing was
> Adam's real intent anyway.
> - I will go through an make sure all existing code is in-line with the
> checkstyle form that we will create. That will require very loud
> music and good drinks but whatever - about as much fun as it was
> getting all the licensing squared away.
>
> I noticed that accumulo has this nicely integrated into their build so
> that gives a great example to follow.
>
> Thanks
> Joe
>
> On Tue, Mar 17, 2015 at 10:44 PM, Joe Witt <jo...@gmail.com> wrote:
> > Sean:
> >
> > Nope we're still pretty basic.
> >
> > Thanks
> > Joe
> >
> > On Tue, Mar 17, 2015 at 10:41 PM, Sean Busbey <bu...@cloudera.com>
> wrote:
> >> On Tue, Mar 17, 2015 at 9:32 PM, Joe Witt <jo...@gmail.com> wrote:
> >>
> >>>
> >>>
> >>> On the thread itself: Anyone interested in pushing forward the
> >>> model/changes to get the formatting process smoothed out please do so.
> >>>
> >>>
> >> Do y'all have a QA bot yet? I'm looking to coalesce the pre-commit
> testing
> >> of Hadoop and HBase in the next ~2-4 weeks. Having a third unrelated
> >> project to throw against that would help me ensure I have something
> >> reusable that can spread across ASF projects.
> >>
> >> We haven't determined yet where the shared pre-commit checker will live,
> >> but we don't seem too opinionated yet so it's unlikely we'll need lots
> of
> >> changes.
> >>
> >> --
> >> Sean
>
Re: lack of consistent formatting - how do others clean this up?
Posted by Joe Witt <jo...@gmail.com>.
Folks,
Benson put in a ticket a while back:
https://issues.apache.org/jira/browse/NIFI-271 to make a DRY
nifi-parent we can extend from in the main nifi line and the
nifi-nar-plugin.
Proposal:
1) Do what Benson said.
2) In that nifi-parent ensure checkstyle is always run and thus
consistent across any nifi item. Fail the build if any violations.
3) In that nifi-parent ensure check-licenses is always run and if any
fails - fail the build.
Commentary:
- This is not as forgiving as Sean suggested but it also does not
preclude us from doing the QC bot to check higher order items.
- This is more in-line with Adam's suggestion but gives the
contributor direct feedback on what is wrong that they can resolve on
their own without us rejecting their PR. This I am guessing was
Adam's real intent anyway.
- I will go through an make sure all existing code is in-line with the
checkstyle form that we will create. That will require very loud
music and good drinks but whatever - about as much fun as it was
getting all the licensing squared away.
I noticed that accumulo has this nicely integrated into their build so
that gives a great example to follow.
Thanks
Joe
On Tue, Mar 17, 2015 at 10:44 PM, Joe Witt <jo...@gmail.com> wrote:
> Sean:
>
> Nope we're still pretty basic.
>
> Thanks
> Joe
>
> On Tue, Mar 17, 2015 at 10:41 PM, Sean Busbey <bu...@cloudera.com> wrote:
>> On Tue, Mar 17, 2015 at 9:32 PM, Joe Witt <jo...@gmail.com> wrote:
>>
>>>
>>>
>>> On the thread itself: Anyone interested in pushing forward the
>>> model/changes to get the formatting process smoothed out please do so.
>>>
>>>
>> Do y'all have a QA bot yet? I'm looking to coalesce the pre-commit testing
>> of Hadoop and HBase in the next ~2-4 weeks. Having a third unrelated
>> project to throw against that would help me ensure I have something
>> reusable that can spread across ASF projects.
>>
>> We haven't determined yet where the shared pre-commit checker will live,
>> but we don't seem too opinionated yet so it's unlikely we'll need lots of
>> changes.
>>
>> --
>> Sean
Re: lack of consistent formatting - how do others clean this up?
Posted by Joe Witt <jo...@gmail.com>.
Sean:
Nope we're still pretty basic.
Thanks
Joe
On Tue, Mar 17, 2015 at 10:41 PM, Sean Busbey <bu...@cloudera.com> wrote:
> On Tue, Mar 17, 2015 at 9:32 PM, Joe Witt <jo...@gmail.com> wrote:
>
>>
>>
>> On the thread itself: Anyone interested in pushing forward the
>> model/changes to get the formatting process smoothed out please do so.
>>
>>
> Do y'all have a QA bot yet? I'm looking to coalesce the pre-commit testing
> of Hadoop and HBase in the next ~2-4 weeks. Having a third unrelated
> project to throw against that would help me ensure I have something
> reusable that can spread across ASF projects.
>
> We haven't determined yet where the shared pre-commit checker will live,
> but we don't seem too opinionated yet so it's unlikely we'll need lots of
> changes.
>
> --
> Sean
Re: lack of consistent formatting - how do others clean this up?
Posted by Sean Busbey <bu...@cloudera.com>.
On Tue, Mar 17, 2015 at 9:32 PM, Joe Witt <jo...@gmail.com> wrote:
>
>
> On the thread itself: Anyone interested in pushing forward the
> model/changes to get the formatting process smoothed out please do so.
>
>
Do y'all have a QA bot yet? I'm looking to coalesce the pre-commit testing
of Hadoop and HBase in the next ~2-4 weeks. Having a third unrelated
project to throw against that would help me ensure I have something
reusable that can spread across ASF projects.
We haven't determined yet where the shared pre-commit checker will live,
but we don't seem too opinionated yet so it's unlikely we'll need lots of
changes.
--
Sean
Re: lack of consistent formatting - how do others clean this up?
Posted by Joe Witt <jo...@gmail.com>.
<soapbox>
"If a pull request doesn't conform to the declared standard, reject it
quickly and move on. "
I understand the spirit of that statement though I see our job of
building a community requiring a more accommodating culture both in
word and deed.
The Robustness Principle is much more appropriate.
http://en.wikipedia.org/wiki/Robustness_principle
It is frankly an honor for the project that someone would take the
time to contribute a PR. Our focus really needs to be on how we find
ways to get it accepted. I agree standards are a must. And we should
have a high bar. But we should also see ourselves as having a duty to
help bring people along. And honestly, it should be fun for people to
contribute.
Joey E advised me that much of the job of being in the PPMC would be
about helping guide PRs across the finish line. He was so right. And
if you think about it - that is kind of awesome.
</soapbox>
On the thread itself: Anyone interested in pushing forward the
model/changes to get the formatting process smoothed out please do so.
Thanks
Joe
On Tue, Mar 17, 2015 at 10:17 PM, Adam Taft <ad...@adamtaft.com> wrote:
> Ryan,
>
> Yes, exactly, what you said. Specifically, "ensure the standard is
> followed so we can all stop thinking about it." That's exactly my point...
> There is no value added dealing with whitespace noise. It shouldn't be
> worked around or accommodated.
>
> If a pull request doesn't conform to the declared standard, reject it
> quickly and move on. If a committer merges poorly formatted code, it's on
> them to fix it up before pushing to the main branches. Like you said,
> declare a standard and stick with it. There may be places that need to be
> fixed up once said standard is made, but it should smooth itself out over
> time.
>
> The principle here is minimizing the number of lines that have changed
> between two commits, ideally avoiding any file changes that don't add
> value, such as whitespace formats, import reordering, etc. This is good
> developer etiquette.
>
> Adam
>
>
> On Tue, Mar 17, 2015 at 8:50 PM, Ryan Blue <bl...@cloudera.com> wrote:
>
>> On 03/17/2015 01:35 PM, Adam Taft wrote:
>>
>>> Whitespace is a holy war and should be avoided. Concern over two space or
>>> four space indents, tabs, etc. is not relevant and adds no value (concern
>>> with spacing is a form of muda). Diffs and other change evaluations can
>>> ideally be done without care for whitespace. This is an easy option for
>>> most diff tools.
>>>
>>
>> The important thing is that we set a standard and stick to it. Workflows
>> commonly rebase on another branch or merge branches with one another, which
>> is a gigantic pain if there are needless conflicts because of whitespace.
>> And the bigger issue is leaking non-functional changes. A few aren't a
>> terribly big deal, but import reordering and changing indentation are bad.
>>
>> I'm also thinking about what I do for other projects to maintain a
>> consistent version when testing with other projects. In our distribution,
>> we snapshot versions and backport changes primarily with cherry-picking. If
>> those cherry-picks fail and have huge conflicts all because of whitespace
>> or import reordering, we have to do much more work and our confidence is
>> lower that we got the backport done right, without bugs.
>>
>> Sun + 4 space indents seems to be the "norm" for NiFi. I like Sean's
>>> second proposal for using jenkins as a nag toward more consistent
>>> formatting. There are code formatting options in each IDE to help as
>>> well.
>>>
>>
>> +1 to nagging as soon as possible
>>
>> But ultimately, I don't think a lot of effort should be spent on this.
>>> Developers should just be mindful, as with any other type of contribution,
>>> that they should conform to the lay of the land.
>>>
>>
>> I agree in principal, but the way to make this happen is to set a standard
>> and stick to it. Calling each other on this in code reviews or
>> automatically will ensure the standard is followed so we can all stop
>> thinking about it.
>>
>> I personally prefer tabs over spaces for my indents (I could rant why this
>>> is "correct" -- again, holy war). But if I'm contributing to NiFi, I'm
>>> using 4 space indents, because that's what is normal for the project.
>>>
>>> Two cents.
>>>
>>> Adam
>>>
>>>
>>> On Tue, Mar 17, 2015 at 2:29 PM, Sean Busbey <bu...@cloudera.com> wrote:
>>>
>>> Set a coding standard (sun + 2 space indents is common), then use
>>>> checkstyle to complain when things don't match. It helps when you can
>>>> provide IDE formatting configurations that match your chosen coding
>>>> standard.
>>>>
>>>
>> For what it's worth, this is what I see on most of the Apache projects
>> that I work on. It is slightly more economical if we introduce a rule about
>> line maximums.
>>
>> And speaking of line limits, I think that's one where we don't want to
>> have automated enforcement. There's a good argument for it (for vertically
>> split editors) but I'd rather a line go a little long because of a change
>> than have that change affect the lines around it. Developer discretion
>> there.
>>
>>
>> Some of the bigger projects use a jenkins precommit bot that checks (among
>>>> other things) that the checkstyle violations don't go up with a proposed
>>>> patch. The bot comments on the jira and the committers decide if any
>>>> violations need to get fixed. Personally, I like this approach better
>>>> than
>>>> e.g. trying to fail the build with checkstyle because it allows
>>>> incremental
>>>> improvement.
>>>>
>>>
>> rb
>>
>> --
>> Ryan Blue
>> Software Engineer
>> Cloudera, Inc.
>>
Re: lack of consistent formatting - how do others clean this up?
Posted by Adam Taft <ad...@adamtaft.com>.
Ryan,
Yes, exactly, what you said. Specifically, "ensure the standard is
followed so we can all stop thinking about it." That's exactly my point...
There is no value added dealing with whitespace noise. It shouldn't be
worked around or accommodated.
If a pull request doesn't conform to the declared standard, reject it
quickly and move on. If a committer merges poorly formatted code, it's on
them to fix it up before pushing to the main branches. Like you said,
declare a standard and stick with it. There may be places that need to be
fixed up once said standard is made, but it should smooth itself out over
time.
The principle here is minimizing the number of lines that have changed
between two commits, ideally avoiding any file changes that don't add
value, such as whitespace formats, import reordering, etc. This is good
developer etiquette.
Adam
On Tue, Mar 17, 2015 at 8:50 PM, Ryan Blue <bl...@cloudera.com> wrote:
> On 03/17/2015 01:35 PM, Adam Taft wrote:
>
>> Whitespace is a holy war and should be avoided. Concern over two space or
>> four space indents, tabs, etc. is not relevant and adds no value (concern
>> with spacing is a form of muda). Diffs and other change evaluations can
>> ideally be done without care for whitespace. This is an easy option for
>> most diff tools.
>>
>
> The important thing is that we set a standard and stick to it. Workflows
> commonly rebase on another branch or merge branches with one another, which
> is a gigantic pain if there are needless conflicts because of whitespace.
> And the bigger issue is leaking non-functional changes. A few aren't a
> terribly big deal, but import reordering and changing indentation are bad.
>
> I'm also thinking about what I do for other projects to maintain a
> consistent version when testing with other projects. In our distribution,
> we snapshot versions and backport changes primarily with cherry-picking. If
> those cherry-picks fail and have huge conflicts all because of whitespace
> or import reordering, we have to do much more work and our confidence is
> lower that we got the backport done right, without bugs.
>
> Sun + 4 space indents seems to be the "norm" for NiFi. I like Sean's
>> second proposal for using jenkins as a nag toward more consistent
>> formatting. There are code formatting options in each IDE to help as
>> well.
>>
>
> +1 to nagging as soon as possible
>
> But ultimately, I don't think a lot of effort should be spent on this.
>> Developers should just be mindful, as with any other type of contribution,
>> that they should conform to the lay of the land.
>>
>
> I agree in principal, but the way to make this happen is to set a standard
> and stick to it. Calling each other on this in code reviews or
> automatically will ensure the standard is followed so we can all stop
> thinking about it.
>
> I personally prefer tabs over spaces for my indents (I could rant why this
>> is "correct" -- again, holy war). But if I'm contributing to NiFi, I'm
>> using 4 space indents, because that's what is normal for the project.
>>
>> Two cents.
>>
>> Adam
>>
>>
>> On Tue, Mar 17, 2015 at 2:29 PM, Sean Busbey <bu...@cloudera.com> wrote:
>>
>> Set a coding standard (sun + 2 space indents is common), then use
>>> checkstyle to complain when things don't match. It helps when you can
>>> provide IDE formatting configurations that match your chosen coding
>>> standard.
>>>
>>
> For what it's worth, this is what I see on most of the Apache projects
> that I work on. It is slightly more economical if we introduce a rule about
> line maximums.
>
> And speaking of line limits, I think that's one where we don't want to
> have automated enforcement. There's a good argument for it (for vertically
> split editors) but I'd rather a line go a little long because of a change
> than have that change affect the lines around it. Developer discretion
> there.
>
>
> Some of the bigger projects use a jenkins precommit bot that checks (among
>>> other things) that the checkstyle violations don't go up with a proposed
>>> patch. The bot comments on the jira and the committers decide if any
>>> violations need to get fixed. Personally, I like this approach better
>>> than
>>> e.g. trying to fail the build with checkstyle because it allows
>>> incremental
>>> improvement.
>>>
>>
> rb
>
> --
> Ryan Blue
> Software Engineer
> Cloudera, Inc.
>
Re: lack of consistent formatting - how do others clean this up?
Posted by Ryan Blue <bl...@cloudera.com>.
On 03/17/2015 01:35 PM, Adam Taft wrote:
> Whitespace is a holy war and should be avoided. Concern over two space or
> four space indents, tabs, etc. is not relevant and adds no value (concern
> with spacing is a form of muda). Diffs and other change evaluations can
> ideally be done without care for whitespace. This is an easy option for
> most diff tools.
The important thing is that we set a standard and stick to it. Workflows
commonly rebase on another branch or merge branches with one another,
which is a gigantic pain if there are needless conflicts because of
whitespace. And the bigger issue is leaking non-functional changes. A
few aren't a terribly big deal, but import reordering and changing
indentation are bad.
I'm also thinking about what I do for other projects to maintain a
consistent version when testing with other projects. In our
distribution, we snapshot versions and backport changes primarily with
cherry-picking. If those cherry-picks fail and have huge conflicts all
because of whitespace or import reordering, we have to do much more work
and our confidence is lower that we got the backport done right, without
bugs.
> Sun + 4 space indents seems to be the "norm" for NiFi. I like Sean's
> second proposal for using jenkins as a nag toward more consistent
> formatting. There are code formatting options in each IDE to help as well.
+1 to nagging as soon as possible
> But ultimately, I don't think a lot of effort should be spent on this.
> Developers should just be mindful, as with any other type of contribution,
> that they should conform to the lay of the land.
I agree in principal, but the way to make this happen is to set a
standard and stick to it. Calling each other on this in code reviews or
automatically will ensure the standard is followed so we can all stop
thinking about it.
> I personally prefer tabs over spaces for my indents (I could rant why this
> is "correct" -- again, holy war). But if I'm contributing to NiFi, I'm
> using 4 space indents, because that's what is normal for the project.
>
> Two cents.
>
> Adam
>
>
> On Tue, Mar 17, 2015 at 2:29 PM, Sean Busbey <bu...@cloudera.com> wrote:
>
>> Set a coding standard (sun + 2 space indents is common), then use
>> checkstyle to complain when things don't match. It helps when you can
>> provide IDE formatting configurations that match your chosen coding
>> standard.
For what it's worth, this is what I see on most of the Apache projects
that I work on. It is slightly more economical if we introduce a rule
about line maximums.
And speaking of line limits, I think that's one where we don't want to
have automated enforcement. There's a good argument for it (for
vertically split editors) but I'd rather a line go a little long because
of a change than have that change affect the lines around it. Developer
discretion there.
>> Some of the bigger projects use a jenkins precommit bot that checks (among
>> other things) that the checkstyle violations don't go up with a proposed
>> patch. The bot comments on the jira and the committers decide if any
>> violations need to get fixed. Personally, I like this approach better than
>> e.g. trying to fail the build with checkstyle because it allows incremental
>> improvement.
rb
--
Ryan Blue
Software Engineer
Cloudera, Inc.
Re: lack of consistent formatting - how do others clean this up?
Posted by Dan Bress <db...@onyxconsults.com>.
+1 for having a consistent style if that will help produce clear and meaningful diffs
+1 if that style can be implemented in eclipse's code formatter, so I can use contrl-shift-f to format my code according to the standard
+0 on what style we use, and whether we use tabs or spaces
+1 as long as the style allows for more than 80 characters on a line(I vote for 120)
+1 for Jenkins nagging me if i check in code that isn't formatted properly
Dan Bress
Software Engineer
ONYX Consulting Services
________________________________________
From: Adam Taft <ad...@adamtaft.com>
Sent: Tuesday, March 17, 2015 4:35 PM
To: dev@nifi.incubator.apache.org
Subject: Re: lack of consistent formatting - how do others clean this up?
Whitespace is a holy war and should be avoided. Concern over two space or
four space indents, tabs, etc. is not relevant and adds no value (concern
with spacing is a form of muda). Diffs and other change evaluations can
ideally be done without care for whitespace. This is an easy option for
most diff tools.
Sun + 4 space indents seems to be the "norm" for NiFi. I like Sean's
second proposal for using jenkins as a nag toward more consistent
formatting. There are code formatting options in each IDE to help as well.
But ultimately, I don't think a lot of effort should be spent on this.
Developers should just be mindful, as with any other type of contribution,
that they should conform to the lay of the land.
I personally prefer tabs over spaces for my indents (I could rant why this
is "correct" -- again, holy war). But if I'm contributing to NiFi, I'm
using 4 space indents, because that's what is normal for the project.
Two cents.
Adam
On Tue, Mar 17, 2015 at 2:29 PM, Sean Busbey <bu...@cloudera.com> wrote:
> Set a coding standard (sun + 2 space indents is common), then use
> checkstyle to complain when things don't match. It helps when you can
> provide IDE formatting configurations that match your chosen coding
> standard.
>
> Some of the bigger projects use a jenkins precommit bot that checks (among
> other things) that the checkstyle violations don't go up with a proposed
> patch. The bot comments on the jira and the committers decide if any
> violations need to get fixed. Personally, I like this approach better than
> e.g. trying to fail the build with checkstyle because it allows incremental
> improvement.
>
> On Tue, Mar 17, 2015 at 1:02 PM, Joe Witt <jo...@gmail.com> wrote:
>
> > Folks,
> >
> > It was brought up to me by a contributor the other day that our lack
> > of consistent formatting makes reviewing true changes difficult. I
> > see it myself quite often. There are difference between formatting in
> > Eclipse/Netbeans/manual VI/etc..
> >
> > I am assuming there are good plugins to just automate this sort of
> > stuff. What do you all think as a good way to go about this?
> >
> > Thanks
> > Joe
> >
>
>
>
> --
> Sean
>
Re: lack of consistent formatting - how do others clean this up?
Posted by Adam Taft <ad...@adamtaft.com>.
Whitespace is a holy war and should be avoided. Concern over two space or
four space indents, tabs, etc. is not relevant and adds no value (concern
with spacing is a form of muda). Diffs and other change evaluations can
ideally be done without care for whitespace. This is an easy option for
most diff tools.
Sun + 4 space indents seems to be the "norm" for NiFi. I like Sean's
second proposal for using jenkins as a nag toward more consistent
formatting. There are code formatting options in each IDE to help as well.
But ultimately, I don't think a lot of effort should be spent on this.
Developers should just be mindful, as with any other type of contribution,
that they should conform to the lay of the land.
I personally prefer tabs over spaces for my indents (I could rant why this
is "correct" -- again, holy war). But if I'm contributing to NiFi, I'm
using 4 space indents, because that's what is normal for the project.
Two cents.
Adam
On Tue, Mar 17, 2015 at 2:29 PM, Sean Busbey <bu...@cloudera.com> wrote:
> Set a coding standard (sun + 2 space indents is common), then use
> checkstyle to complain when things don't match. It helps when you can
> provide IDE formatting configurations that match your chosen coding
> standard.
>
> Some of the bigger projects use a jenkins precommit bot that checks (among
> other things) that the checkstyle violations don't go up with a proposed
> patch. The bot comments on the jira and the committers decide if any
> violations need to get fixed. Personally, I like this approach better than
> e.g. trying to fail the build with checkstyle because it allows incremental
> improvement.
>
> On Tue, Mar 17, 2015 at 1:02 PM, Joe Witt <jo...@gmail.com> wrote:
>
> > Folks,
> >
> > It was brought up to me by a contributor the other day that our lack
> > of consistent formatting makes reviewing true changes difficult. I
> > see it myself quite often. There are difference between formatting in
> > Eclipse/Netbeans/manual VI/etc..
> >
> > I am assuming there are good plugins to just automate this sort of
> > stuff. What do you all think as a good way to go about this?
> >
> > Thanks
> > Joe
> >
>
>
>
> --
> Sean
>
Re: lack of consistent formatting - how do others clean this up?
Posted by Sean Busbey <bu...@cloudera.com>.
Set a coding standard (sun + 2 space indents is common), then use
checkstyle to complain when things don't match. It helps when you can
provide IDE formatting configurations that match your chosen coding
standard.
Some of the bigger projects use a jenkins precommit bot that checks (among
other things) that the checkstyle violations don't go up with a proposed
patch. The bot comments on the jira and the committers decide if any
violations need to get fixed. Personally, I like this approach better than
e.g. trying to fail the build with checkstyle because it allows incremental
improvement.
On Tue, Mar 17, 2015 at 1:02 PM, Joe Witt <jo...@gmail.com> wrote:
> Folks,
>
> It was brought up to me by a contributor the other day that our lack
> of consistent formatting makes reviewing true changes difficult. I
> see it myself quite often. There are difference between formatting in
> Eclipse/Netbeans/manual VI/etc..
>
> I am assuming there are good plugins to just automate this sort of
> stuff. What do you all think as a good way to go about this?
>
> Thanks
> Joe
>
--
Sean