You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@hbase.apache.org by Josh Elser <el...@apache.org> on 2022/03/20 20:50:16 UTC
Re: A tweak to our checkstyle configuration
Going through my inbox...
1. Great to have tooling which can validate (and fix) code which is not
currently to style.
2. I would prefer a style guide (such as Google's Java Style) which is
"generally accepted" by the Java industry at large and we can use as-is.
However, I don't feel strongly on this.
3. I have no objections to Nick's original ask to allow one-line if
blocks on the same line of code with brackets.
4. I prefer brackets for one line if/else blocks over no-brackets for
the same (as Andrew indicates about avoiding dangling if-else blocks),
but would not -1 a change if the majority felt otherwise.
On 1/16/22 3:01 AM, 张铎(Duo Zhang) wrote:
> On enforcing the coding standards, I've filed HBASE-26617, to introduce the
> spotless plugin to HBase.
>
> We can add 'mvn spotless:check' to our pre commit checks, so we can
> enforce the coding standards.
>
> And 'mvn spotless:apply' will format everything for you.
>
> Andrew Purtell <an...@gmail.com> 于2022年1月16日周日 07:39写道：
>
>> There are a handful of anti patterns to avoid, like dangling if-elses.
>> (Always use braces around code blocks!) Otherwise we have been following
>> the Java basic guidelines with modifications for indent width and maximum
>> line length and I see no pressing reason why this needs to change. Happy
>> with the status quo. That said I see no reason to reject Nicks’s small
>> proposed changes. We definitely don’t need to adopt a totally different
>> style guide in response to a modest proposal. This seems out of proportion
>> to the ask.
>>
>> If we are going to change checkstyle rules it would be necessary for the
>> proposer to provide a linter for the rest of us to use as well as a Yetus
>> precommit phase that implements the checks. Otherwise it would be a half
>> completed proposal and worse than making no changes. Please also provide
>> HOWTOs for configuring the IDEA and Eclipse IDEs.
>>
>>> On Jan 15, 2022, at 1:07 AM, 张铎 <pa...@gmail.com> wrote:
>>>
>>> What about just switching to use google java style?
>>>
>>> Nick Dimiduk <nd...@apache.org> 于2022年1月13日周四 03:22写道：
>>>
>>>> Hey all.
>>>>
>>>> Discussion on the PR has resulted in an impasse of opinion, but also
>>>> renewed interest in improvements to static analysis in general
>>>> (HBASE-26617).
>>>>
>>>> I think that this kind of code hygiene is very important for the
>> long-term
>>>> maintenance of a large project like ours and especially one that accepts
>>>> contributions from a broad audience. I would really appreciate it if
>> some
>>>> more folks would chime into these discussions on PRs, or bring your
>>>> concerns back up to this thread. I'm game to help see the work done,
>> but we
>>>> need more voices to participate in defining what is required by the
>>>> community.
>>>>
>>>> Thanks in advance,
>>>> Nick
>>>>
>>>>> On Thu, Dec 9, 2021 at 3:58 PM Nick Dimiduk <nd...@apache.org>
>> wrote:
>>>>>
>>>>> Heya,
>>>>>
>>>>> I have posted a small change to our checkstyle configuration on
>>>>> HBASE-26536. This change will relax the whitespace rules regarding the
>>>>> left-curly-bracket ('{') character. Specifically, I intend this change
>> to
>>>>> allow short expressions that include a nested scope that fits entirely
>> on
>>>>> one line. The example I provide is:
>>>>>
>>>>> if (foo == null) { return null; }
>>>>>
>>>>> This whitespace style is already present (though I think not in popular
>>>>> usage) within the codebase. Please take a look and let me know if you
>>>> have
>>>>> any concerns about making this change.
>>>>>
>>>>> Thanks,
>>>>> Nick
>>>>>
>>>>> https://issues.apache.org/jira/browse/HBASE-26536
>>>>> https://github.com/apache/hbase/pull/3913
>>>>>
>>>>
>>
>
Re: A tweak to our checkstyle configuration
Posted by "张铎(Duo Zhang)" <pa...@gmail.com>.
So for #1, would you like to take a look at
https://github.com/apache/hbase/pull/4214
:)
Josh Elser <el...@apache.org> 于2022年3月21日周一 04:50写道：
> Going through my inbox...
>
> 1. Great to have tooling which can validate (and fix) code which is not
> currently to style.
> 2. I would prefer a style guide (such as Google's Java Style) which is
> "generally accepted" by the Java industry at large and we can use as-is.
> However, I don't feel strongly on this.
> 3. I have no objections to Nick's original ask to allow one-line if
> blocks on the same line of code with brackets.
> 4. I prefer brackets for one line if/else blocks over no-brackets for
> the same (as Andrew indicates about avoiding dangling if-else blocks),
> but would not -1 a change if the majority felt otherwise.
>
> On 1/16/22 3:01 AM, 张铎(Duo Zhang) wrote:
> > On enforcing the coding standards, I've filed HBASE-26617, to introduce
> the
> > spotless plugin to HBase.
> >
> > We can add 'mvn spotless:check' to our pre commit checks, so we can
> > enforce the coding standards.
> >
> > And 'mvn spotless:apply' will format everything for you.
> >
> > Andrew Purtell <an...@gmail.com> 于2022年1月16日周日 07:39写道：
> >
> >> There are a handful of anti patterns to avoid, like dangling if-elses.
> >> (Always use braces around code blocks!) Otherwise we have been following
> >> the Java basic guidelines with modifications for indent width and
> maximum
> >> line length and I see no pressing reason why this needs to change. Happy
> >> with the status quo. That said I see no reason to reject Nicks’s small
> >> proposed changes. We definitely don’t need to adopt a totally different
> >> style guide in response to a modest proposal. This seems out of
> proportion
> >> to the ask.
> >>
> >> If we are going to change checkstyle rules it would be necessary for the
> >> proposer to provide a linter for the rest of us to use as well as a
> Yetus
> >> precommit phase that implements the checks. Otherwise it would be a half
> >> completed proposal and worse than making no changes. Please also provide
> >> HOWTOs for configuring the IDEA and Eclipse IDEs.
> >>
> >>> On Jan 15, 2022, at 1:07 AM, 张铎 <pa...@gmail.com> wrote:
> >>>
> >>> What about just switching to use google java style?
> >>>
> >>> Nick Dimiduk <nd...@apache.org> 于2022年1月13日周四 03:22写道：
> >>>
> >>>> Hey all.
> >>>>
> >>>> Discussion on the PR has resulted in an impasse of opinion, but also
> >>>> renewed interest in improvements to static analysis in general
> >>>> (HBASE-26617).
> >>>>
> >>>> I think that this kind of code hygiene is very important for the
> >> long-term
> >>>> maintenance of a large project like ours and especially one that
> accepts
> >>>> contributions from a broad audience. I would really appreciate it if
> >> some
> >>>> more folks would chime into these discussions on PRs, or bring your
> >>>> concerns back up to this thread. I'm game to help see the work done,
> >> but we
> >>>> need more voices to participate in defining what is required by the
> >>>> community.
> >>>>
> >>>> Thanks in advance,
> >>>> Nick
> >>>>
> >>>>> On Thu, Dec 9, 2021 at 3:58 PM Nick Dimiduk <nd...@apache.org>
> >> wrote:
> >>>>>
> >>>>> Heya,
> >>>>>
> >>>>> I have posted a small change to our checkstyle configuration on
> >>>>> HBASE-26536. This change will relax the whitespace rules regarding
> the
> >>>>> left-curly-bracket ('{') character. Specifically, I intend this
> change
> >> to
> >>>>> allow short expressions that include a nested scope that fits
> entirely
> >> on
> >>>>> one line. The example I provide is:
> >>>>>
> >>>>> if (foo == null) { return null; }
> >>>>>
> >>>>> This whitespace style is already present (though I think not in
> popular
> >>>>> usage) within the codebase. Please take a look and let me know if you
> >>>> have
> >>>>> any concerns about making this change.
> >>>>>
> >>>>> Thanks,
> >>>>> Nick
> >>>>>
> >>>>> https://issues.apache.org/jira/browse/HBASE-26536
> >>>>> https://github.com/apache/hbase/pull/3913
> >>>>>
> >>>>
> >>
> >
>