[DISCUSS] update our imports layout

[Chia-Ping Tsai <ch...@apache.org>]

hi folks,

I noticed the code conflict occurs on the imports frequently. To resolve the conflict is a complete waste of time, so i feel it is time to update our imports layout and hold our attraction on it.

The import layout is shown below. (see hbase_eclipse_formatter.xml)
javax.*
blank line
java.*
blank line
import all other imports
blank line
import static all other imports

Q1:
As i see it, two updates should be considered.
1) Should we move the "import static" to the top? (It seems most of files have its static imports on the top)
2) Should we move the shaded class into new blocks?

The new layout looks like this.
import static all other imports
blank line
javax.*
blank line
java.*
blank line
org.*
blank line
org.apache.hadoop.hbase.shaded.*
blank line
import all other imports

Q2:
Should we check the import layout before committing? Perhaps we can address this in the HBASE-18438. The issue try to add the check of unused imports

Any suggestions? Thanks.
--
Chia-Ping

Re: [DISCUSS] update our imports layout

[Ted Yu <yu...@gmail.com>]

+1 on not using wildcard import .

On Sun, Oct 1, 2017 at 10:24 AM, Peter Somogyi <ps...@cloudera.com>
wrote:

> I like the layout you suggested Chia-Ping and also to check this in the
> precommit run.
> Shall we also add "not to use * imports" to the verification?
>
> On Sun, Oct 1, 2017 at 9:09 AM, Chia-Ping Tsai <ch...@apache.org>
> wrote:
>
> > bq.  I guess you meant attention.
> > You are right. sorry for the misspelling. ☹
> >
> > On 2017-10-01 23:33, Ted Yu <yu...@gmail.com> wrote:
> > > bq. hold our attraction
> > >
> > > I guess you meant attention.
> > >
> > > The suggestions under Q1 are good.
> > >
> > >
> > >
> > > On Sun, Oct 1, 2017 at 8:27 AM, Chia-Ping Tsai <ch...@apache.org>
> > wrote:
> > >
> > > > hi folks,
> > > >
> > > > I noticed the code conflict occurs on the imports frequently. To
> > resolve
> > > > the conflict is a complete waste of time, so i feel it is time to
> > update
> > > > our imports layout and hold our attraction on it.
> > > >
> > > > The import layout is shown below. (see hbase_eclipse_formatter.xml)
> > > > javax.*
> > > > blank line
> > > > java.*
> > > > blank line
> > > > import all other imports
> > > > blank line
> > > > import static all other imports
> > > >
> > > > Q1:
> > > > As i see it, two updates should be considered.
> > > > 1) Should we move the "import static" to the top? (It seems most of
> > files
> > > > have its static imports on the top)
> > > > 2) Should we move the shaded class into new blocks?
> > > >
> > > > The new layout looks like this.
> > > > import static all other imports
> > > > blank line
> > > > javax.*
> > > > blank line
> > > > java.*
> > > > blank line
> > > > org.*
> > > > blank line
> > > > org.apache.hadoop.hbase.shaded.*
> > > > blank line
> > > > import all other imports
> > > >
> > > > Q2:
> > > > Should we check the import layout before committing? Perhaps we can
> > > > address this in the HBASE-18438. The issue try to add the check of
> > unused
> > > > imports
> > > >
> > > > Any suggestions? Thanks.
> > > > --
> > > > Chia-Ping
> > > >
> > > >
> > >
> >
>

Re: [DISCUSS] update our imports layout

[Peter Somogyi <ps...@cloudera.com>]

I like the layout you suggested Chia-Ping and also to check this in the
precommit run.
Shall we also add "not to use * imports" to the verification?

On Sun, Oct 1, 2017 at 9:09 AM, Chia-Ping Tsai <ch...@apache.org> wrote:

> bq.  I guess you meant attention.
> You are right. sorry for the misspelling. ☹
>
> On 2017-10-01 23:33, Ted Yu <yu...@gmail.com> wrote:
> > bq. hold our attraction
> >
> > I guess you meant attention.
> >
> > The suggestions under Q1 are good.
> >
> >
> >
> > On Sun, Oct 1, 2017 at 8:27 AM, Chia-Ping Tsai <ch...@apache.org>
> wrote:
> >
> > > hi folks,
> > >
> > > I noticed the code conflict occurs on the imports frequently. To
> resolve
> > > the conflict is a complete waste of time, so i feel it is time to
> update
> > > our imports layout and hold our attraction on it.
> > >
> > > The import layout is shown below. (see hbase_eclipse_formatter.xml)
> > > javax.*
> > > blank line
> > > java.*
> > > blank line
> > > import all other imports
> > > blank line
> > > import static all other imports
> > >
> > > Q1:
> > > As i see it, two updates should be considered.
> > > 1) Should we move the "import static" to the top? (It seems most of
> files
> > > have its static imports on the top)
> > > 2) Should we move the shaded class into new blocks?
> > >
> > > The new layout looks like this.
> > > import static all other imports
> > > blank line
> > > javax.*
> > > blank line
> > > java.*
> > > blank line
> > > org.*
> > > blank line
> > > org.apache.hadoop.hbase.shaded.*
> > > blank line
> > > import all other imports
> > >
> > > Q2:
> > > Should we check the import layout before committing? Perhaps we can
> > > address this in the HBASE-18438. The issue try to add the check of
> unused
> > > imports
> > >
> > > Any suggestions? Thanks.
> > > --
> > > Chia-Ping
> > >
> > >
> >
>

Re: [DISCUSS] update our imports layout

[Chia-Ping Tsai <ch...@apache.org>]

bq.  I guess you meant attention.
You are right. sorry for the misspelling. ☹

On 2017-10-01 23:33, Ted Yu <yu...@gmail.com> wrote: 
> bq. hold our attraction
> 
> I guess you meant attention.
> 
> The suggestions under Q1 are good.
> 
> 
> 
> On Sun, Oct 1, 2017 at 8:27 AM, Chia-Ping Tsai <ch...@apache.org> wrote:
> 
> > hi folks,
> >
> > I noticed the code conflict occurs on the imports frequently. To resolve
> > the conflict is a complete waste of time, so i feel it is time to update
> > our imports layout and hold our attraction on it.
> >
> > The import layout is shown below. (see hbase_eclipse_formatter.xml)
> > javax.*
> > blank line
> > java.*
> > blank line
> > import all other imports
> > blank line
> > import static all other imports
> >
> > Q1:
> > As i see it, two updates should be considered.
> > 1) Should we move the "import static" to the top? (It seems most of files
> > have its static imports on the top)
> > 2) Should we move the shaded class into new blocks?
> >
> > The new layout looks like this.
> > import static all other imports
> > blank line
> > javax.*
> > blank line
> > java.*
> > blank line
> > org.*
> > blank line
> > org.apache.hadoop.hbase.shaded.*
> > blank line
> > import all other imports
> >
> > Q2:
> > Should we check the import layout before committing? Perhaps we can
> > address this in the HBASE-18438. The issue try to add the check of unused
> > imports
> >
> > Any suggestions? Thanks.
> > --
> > Chia-Ping
> >
> >
> 

Re: [DISCUSS] update our imports layout

[Ted Yu <yu...@gmail.com>]

bq. hold our attraction

I guess you meant attention.

The suggestions under Q1 are good.



On Sun, Oct 1, 2017 at 8:27 AM, Chia-Ping Tsai <ch...@apache.org> wrote:

> hi folks,
>
> I noticed the code conflict occurs on the imports frequently. To resolve
> the conflict is a complete waste of time, so i feel it is time to update
> our imports layout and hold our attraction on it.
>
> The import layout is shown below. (see hbase_eclipse_formatter.xml)
> javax.*
> blank line
> java.*
> blank line
> import all other imports
> blank line
> import static all other imports
>
> Q1:
> As i see it, two updates should be considered.
> 1) Should we move the "import static" to the top? (It seems most of files
> have its static imports on the top)
> 2) Should we move the shaded class into new blocks?
>
> The new layout looks like this.
> import static all other imports
> blank line
> javax.*
> blank line
> java.*
> blank line
> org.*
> blank line
> org.apache.hadoop.hbase.shaded.*
> blank line
> import all other imports
>
> Q2:
> Should we check the import layout before committing? Perhaps we can
> address this in the HBASE-18438. The issue try to add the check of unused
> imports
>
> Any suggestions? Thanks.
> --
> Chia-Ping
>
>

Re: [DISCUSS] update our imports layout

[Chia-Ping Tsai <ch...@apache.org>]

Both of google style or own special way are fine to me. :-)

Another issue is how to make people feel the special import ordering is important? I prefer adding the order check to precommit run.

On 2017-10-02 03:37, Josh Elser <el...@apache.org> wrote: 
> Playing devil's advocate:
> 
> Do we want to maintain our own "special" way of doing imports instead of 
> relying on something such as the Google Java style guide? [1]
> 
> +1 to the idea of cleaning things up, but just curious if people feel 
> like our special import ordering is important (and not just vestigial). 
> Personally, I don't have much concern about import order than having 
> consistency -- I like the idea of letting someone hash out what is 
> recommended :)
> 
> [1] 
> https://google.github.io/styleguide/javaguide.html#s3.3-import-statements
> 
> On 10/1/17 11:27 AM, Chia-Ping Tsai wrote:
> > hi folks,
> > 
> > I noticed the code conflict occurs on the imports frequently. To resolve the conflict is a complete waste of time, so i feel it is time to update our imports layout and hold our attraction on it.
> > 
> > The import layout is shown below. (see hbase_eclipse_formatter.xml)
> > javax.*
> > blank line
> > java.*
> > blank line
> > import all other imports
> > blank line
> > import static all other imports
> > 
> > Q1:
> > As i see it, two updates should be considered.
> > 1) Should we move the "import static" to the top? (It seems most of files have its static imports on the top)
> > 2) Should we move the shaded class into new blocks?
> > 
> > The new layout looks like this.
> > import static all other imports
> > blank line
> > javax.*
> > blank line
> > java.*
> > blank line
> > org.*
> > blank line
> > org.apache.hadoop.hbase.shaded.*
> > blank line
> > import all other imports
> > 
> > Q2:
> > Should we check the import layout before committing? Perhaps we can address this in the HBASE-18438. The issue try to add the check of unused imports
> > 
> > Any suggestions? Thanks.
> > --
> > Chia-Ping
> > 
> 

Re: [DISCUSS] update our imports layout

["张铎 (Duo Zhang)" <pa...@gmail.com>]

I think Google's style is good enough. But for us, I'm +1 on moving the
imports of hbase-thirdparty, i.e., the shaded ones, to a new block.

2017-10-03 0:30 GMT+08:00 Andrew Purtell <ap...@apache.org>:

> I think whatever offers lower overall effort for committers would be the
> better choice. I don't have any particular concern about ordering.
>
> For what it's worth, I am not a fan of wildcard imports and undo them
> wherever I see them. This is just a personal preference though. If we
> officially are going to allow them I can live with it.
>
> Changing the ordering of imports is going to cause a lot of busywork fixing
> up patches and will sit in our history like a singularity, but reversed :-)
> , where any patch outside the event horizon will be clobbered. This will at
> least temporarily raise the work factor experienced by maintainers and
> committers who work with multiple code lines. Speaking of that, if we do it
> on one code line we should do it on them all. I also think any patch which
> does a lot of reordering of imports while also introducing a bug fix or
> feature should be rejected. If imports are to be reordered, it should be
> done in its own change set in isolation.
>
>
> On Sun, Oct 1, 2017 at 12:37 PM, Josh Elser <el...@apache.org> wrote:
>
> > Playing devil's advocate:
> >
> > Do we want to maintain our own "special" way of doing imports instead of
> > relying on something such as the Google Java style guide? [1]
> >
> > +1 to the idea of cleaning things up, but just curious if people feel
> like
> > our special import ordering is important (and not just vestigial).
> > Personally, I don't have much concern about import order than having
> > consistency -- I like the idea of letting someone hash out what is
> > recommended :)
> >
> > [1] https://google.github.io/styleguide/javaguide.html#s3.3-
> > import-statements
> >
> >
> > On 10/1/17 11:27 AM, Chia-Ping Tsai wrote:
> >
> >> hi folks,
> >>
> >> I noticed the code conflict occurs on the imports frequently. To resolve
> >> the conflict is a complete waste of time, so i feel it is time to update
> >> our imports layout and hold our attraction on it.
> >>
> >> The import layout is shown below. (see hbase_eclipse_formatter.xml)
> >> javax.*
> >> blank line
> >> java.*
> >> blank line
> >> import all other imports
> >> blank line
> >> import static all other imports
> >>
> >> Q1:
> >> As i see it, two updates should be considered.
> >> 1) Should we move the "import static" to the top? (It seems most of
> files
> >> have its static imports on the top)
> >> 2) Should we move the shaded class into new blocks?
> >>
> >> The new layout looks like this.
> >> import static all other imports
> >> blank line
> >> javax.*
> >> blank line
> >> java.*
> >> blank line
> >> org.*
> >> blank line
> >> org.apache.hadoop.hbase.shaded.*
> >> blank line
> >> import all other imports
> >>
> >> Q2:
> >> Should we check the import layout before committing? Perhaps we can
> >> address this in the HBASE-18438. The issue try to add the check of
> unused
> >> imports
> >>
> >> Any suggestions? Thanks.
> >> --
> >> Chia-Ping
> >>
> >>
>
>
> --
> Best regards,
> Andrew
>
> Words like orphans lost among the crosstalk, meaning torn from truth's
> decrepit hands
>    - A23, Crosstalk
>

Re: [DISCUSS] update our imports layout

[Andrew Purtell <ap...@apache.org>]

I think whatever offers lower overall effort for committers would be the
better choice. I don't have any particular concern about ordering.

For what it's worth, I am not a fan of wildcard imports and undo them
wherever I see them. This is just a personal preference though. If we
officially are going to allow them I can live with it.

Changing the ordering of imports is going to cause a lot of busywork fixing
up patches and will sit in our history like a singularity, but reversed :-)
, where any patch outside the event horizon will be clobbered. This will at
least temporarily raise the work factor experienced by maintainers and
committers who work with multiple code lines. Speaking of that, if we do it
on one code line we should do it on them all. I also think any patch which
does a lot of reordering of imports while also introducing a bug fix or
feature should be rejected. If imports are to be reordered, it should be
done in its own change set in isolation.


On Sun, Oct 1, 2017 at 12:37 PM, Josh Elser <el...@apache.org> wrote:

> Playing devil's advocate:
>
> Do we want to maintain our own "special" way of doing imports instead of
> relying on something such as the Google Java style guide? [1]
>
> +1 to the idea of cleaning things up, but just curious if people feel like
> our special import ordering is important (and not just vestigial).
> Personally, I don't have much concern about import order than having
> consistency -- I like the idea of letting someone hash out what is
> recommended :)
>
> [1] https://google.github.io/styleguide/javaguide.html#s3.3-
> import-statements
>
>
> On 10/1/17 11:27 AM, Chia-Ping Tsai wrote:
>
>> hi folks,
>>
>> I noticed the code conflict occurs on the imports frequently. To resolve
>> the conflict is a complete waste of time, so i feel it is time to update
>> our imports layout and hold our attraction on it.
>>
>> The import layout is shown below. (see hbase_eclipse_formatter.xml)
>> javax.*
>> blank line
>> java.*
>> blank line
>> import all other imports
>> blank line
>> import static all other imports
>>
>> Q1:
>> As i see it, two updates should be considered.
>> 1) Should we move the "import static" to the top? (It seems most of files
>> have its static imports on the top)
>> 2) Should we move the shaded class into new blocks?
>>
>> The new layout looks like this.
>> import static all other imports
>> blank line
>> javax.*
>> blank line
>> java.*
>> blank line
>> org.*
>> blank line
>> org.apache.hadoop.hbase.shaded.*
>> blank line
>> import all other imports
>>
>> Q2:
>> Should we check the import layout before committing? Perhaps we can
>> address this in the HBASE-18438. The issue try to add the check of unused
>> imports
>>
>> Any suggestions? Thanks.
>> --
>> Chia-Ping
>>
>>


-- 
Best regards,
Andrew

Words like orphans lost among the crosstalk, meaning torn from truth's
decrepit hands
   - A23, Crosstalk

Re: [DISCUSS] update our imports layout

[Josh Elser <el...@apache.org>]

Playing devil's advocate:

Do we want to maintain our own "special" way of doing imports instead of 
relying on something such as the Google Java style guide? [1]

+1 to the idea of cleaning things up, but just curious if people feel 
like our special import ordering is important (and not just vestigial). 
Personally, I don't have much concern about import order than having 
consistency -- I like the idea of letting someone hash out what is 
recommended :)

[1] 
https://google.github.io/styleguide/javaguide.html#s3.3-import-statements

On 10/1/17 11:27 AM, Chia-Ping Tsai wrote:
> hi folks,
> 
> I noticed the code conflict occurs on the imports frequently. To resolve the conflict is a complete waste of time, so i feel it is time to update our imports layout and hold our attraction on it.
> 
> The import layout is shown below. (see hbase_eclipse_formatter.xml)
> javax.*
> blank line
> java.*
> blank line
> import all other imports
> blank line
> import static all other imports
> 
> Q1:
> As i see it, two updates should be considered.
> 1) Should we move the "import static" to the top? (It seems most of files have its static imports on the top)
> 2) Should we move the shaded class into new blocks?
> 
> The new layout looks like this.
> import static all other imports
> blank line
> javax.*
> blank line
> java.*
> blank line
> org.*
> blank line
> org.apache.hadoop.hbase.shaded.*
> blank line
> import all other imports
> 
> Q2:
> Should we check the import layout before committing? Perhaps we can address this in the HBASE-18438. The issue try to add the check of unused imports
> 
> Any suggestions? Thanks.
> --
> Chia-Ping
>