You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@sentry.apache.org by Alexander Kolbasov <ak...@cloudera.com> on 2017/07/07 14:50:20 UTC
Code style and checkstyle
Hello,
What would people thing about enforcing code style for Apache Sentry code?
There is a maven/IntelliJ plugin - checkstyle (
http://checkstyle.sourceforge.net/index.html) that can help enforcing the
style.
The problem is that the current code isn't compliant and fails a lot of
checks and it wouldn't be reasonable to mass-modify a lot of existing code
to comply. Instead I would suggest that the new code and the code that is
modified extensively should comply and we can enforce this at code review
time. It might be interesting to research whether checkstyle tool can be
configured with a list of exclude/include files which can be modified as
more files are made compliant.
Another interesting issue is related to the style details. There are two
styles supported out of the box - original Sun Java style and Google style.
I am not sure what is the difference between the, it would be interesting
to research.
What do you think about this?
- Alex
Re: Code style and checkstyle
Posted by Alexander Kolbasov <ak...@cloudera.com>.
Interesting - I didn't know that I could configure IntelliJ to use the
Google style and automatically format according to it. Will try.
On Mon, Jul 10, 2017 at 7:17 AM, Kalyan Kumar Kalvagadda <
kkalyan@cloudera.com> wrote:
> Sasha,
>
> I have downloaded the google style xml file and added it to intellij.
>
> To change the existing code according to this style I should pick a file
> and apply Code=>Format Code menu option.
>
> Is there something else I should be doing?
>
> -Kalyan
>
> On Fri, Jul 7, 2017 at 4:34 PM, Alexander Kolbasov <ak...@cloudera.com>
> wrote:
>
> > I was able to use Google style in IntelliJ - the trick is to set
> checkstyle
> > version to 7.1 and *apply* the change. After that you can point it to the
> > google style xml file.
> >
> > Should we make this a requirement for new files?
> >
> > On Fri, Jul 7, 2017 at 6:06 PM, Sergio Pena <se...@cloudera.com>
> > wrote:
> >
> > > I really like the google java style. Seems more updated to the
> developing
> > > practices these days.
> > >
> > > On Fri, Jul 7, 2017 at 10:55 AM, Alexander Kolbasov <
> akolb@cloudera.com>
> > > wrote:
> > >
> > > > Thanks Lina. What is AD_SAL and MD_SAL? Who were you citing as the
> > person
> > > > preferring yo use Google Java style? Is there any description of the
> > > > differences between the two?
> > > >
> > > > - Alex
> > > >
> > > > On Fri, Jul 7, 2017 at 5:51 PM, Na Li <li...@cloudera.com> wrote:
> > > >
> > > > > Sasha,
> > > > >
> > > > > found the following online, and I think we can go for google style.
> > > > >
> > > > > "Sun coding style was used in original contribution of Controller
> and
> > > > > AD-SAL and is mostly used in AD-SAL.
> > > > >
> > > > > Google Java Style is used in newer code such as YANGTools, MD-SAL,
> > > etc...
> > > > > so I would say prefer Google Java Style, since community decided on
> > > going
> > > > > with that style."
> > > > >
> > > > > I like the idea of enforcing style on new code. I can take a look
> on
> > > how
> > > > > easy to do that.
> > > > >
> > > > > Thanks,
> > > > >
> > > > > Lina
> > > > >
> > > > > On Fri, Jul 7, 2017 at 9:50 AM, Alexander Kolbasov <
> > akolb@cloudera.com
> > > >
> > > > > wrote:
> > > > >
> > > > > > Hello,
> > > > > >
> > > > > > What would people thing about enforcing code style for Apache
> > Sentry
> > > > > code?
> > > > > > There is a maven/IntelliJ plugin - checkstyle (
> > > > > > http://checkstyle.sourceforge.net/index.html) that can help
> > > enforcing
> > > > > the
> > > > > > style.
> > > > > >
> > > > > > The problem is that the current code isn't compliant and fails a
> > lot
> > > of
> > > > > > checks and it wouldn't be reasonable to mass-modify a lot of
> > existing
> > > > > code
> > > > > > to comply. Instead I would suggest that the new code and the code
> > > that
> > > > is
> > > > > > modified extensively should comply and we can enforce this at
> code
> > > > review
> > > > > > time. It might be interesting to research whether checkstyle tool
> > can
> > > > be
> > > > > > configured with a list of exclude/include files which can be
> > modified
> > > > as
> > > > > > more files are made compliant.
> > > > > >
> > > > > > Another interesting issue is related to the style details. There
> > are
> > > > two
> > > > > > styles supported out of the box - original Sun Java style and
> > Google
> > > > > style.
> > > > > > I am not sure what is the difference between the, it would be
> > > > interesting
> > > > > > to research.
> > > > > >
> > > > > > What do you think about this?
> > > > > >
> > > > > > - Alex
> > > > > >
> > > > >
> > > >
> > >
> >
>
Re: Code style and checkstyle
Posted by Kalyan Kumar Kalvagadda <kk...@cloudera.com>.
Sasha,
Adding to my previous mail
Here are the two things I think should be done on existing code to follow
new google style.
1. Use Code=>Format code option
2. Analyze=>Inspect code
Is there something else we should be doing?
-Kalyan
On Mon, Jul 10, 2017 at 12:17 AM, Kalyan Kumar Kalvagadda <
kkalyan@cloudera.com> wrote:
> Sasha,
>
> I have downloaded the google style xml file and added it to intellij.
>
> To change the existing code according to this style I should pick a file
> and apply Code=>Format Code menu option.
>
> Is there something else I should be doing?
>
> -Kalyan
>
> On Fri, Jul 7, 2017 at 4:34 PM, Alexander Kolbasov <ak...@cloudera.com>
> wrote:
>
>> I was able to use Google style in IntelliJ - the trick is to set
>> checkstyle
>> version to 7.1 and *apply* the change. After that you can point it to the
>> google style xml file.
>>
>> Should we make this a requirement for new files?
>>
>> On Fri, Jul 7, 2017 at 6:06 PM, Sergio Pena <se...@cloudera.com>
>> wrote:
>>
>> > I really like the google java style. Seems more updated to the
>> developing
>> > practices these days.
>> >
>> > On Fri, Jul 7, 2017 at 10:55 AM, Alexander Kolbasov <akolb@cloudera.com
>> >
>> > wrote:
>> >
>> > > Thanks Lina. What is AD_SAL and MD_SAL? Who were you citing as the
>> person
>> > > preferring yo use Google Java style? Is there any description of the
>> > > differences between the two?
>> > >
>> > > - Alex
>> > >
>> > > On Fri, Jul 7, 2017 at 5:51 PM, Na Li <li...@cloudera.com> wrote:
>> > >
>> > > > Sasha,
>> > > >
>> > > > found the following online, and I think we can go for google style.
>> > > >
>> > > > "Sun coding style was used in original contribution of Controller
>> and
>> > > > AD-SAL and is mostly used in AD-SAL.
>> > > >
>> > > > Google Java Style is used in newer code such as YANGTools, MD-SAL,
>> > etc...
>> > > > so I would say prefer Google Java Style, since community decided on
>> > going
>> > > > with that style."
>> > > >
>> > > > I like the idea of enforcing style on new code. I can take a look on
>> > how
>> > > > easy to do that.
>> > > >
>> > > > Thanks,
>> > > >
>> > > > Lina
>> > > >
>> > > > On Fri, Jul 7, 2017 at 9:50 AM, Alexander Kolbasov <
>> akolb@cloudera.com
>> > >
>> > > > wrote:
>> > > >
>> > > > > Hello,
>> > > > >
>> > > > > What would people thing about enforcing code style for Apache
>> Sentry
>> > > > code?
>> > > > > There is a maven/IntelliJ plugin - checkstyle (
>> > > > > http://checkstyle.sourceforge.net/index.html) that can help
>> > enforcing
>> > > > the
>> > > > > style.
>> > > > >
>> > > > > The problem is that the current code isn't compliant and fails a
>> lot
>> > of
>> > > > > checks and it wouldn't be reasonable to mass-modify a lot of
>> existing
>> > > > code
>> > > > > to comply. Instead I would suggest that the new code and the code
>> > that
>> > > is
>> > > > > modified extensively should comply and we can enforce this at code
>> > > review
>> > > > > time. It might be interesting to research whether checkstyle tool
>> can
>> > > be
>> > > > > configured with a list of exclude/include files which can be
>> modified
>> > > as
>> > > > > more files are made compliant.
>> > > > >
>> > > > > Another interesting issue is related to the style details. There
>> are
>> > > two
>> > > > > styles supported out of the box - original Sun Java style and
>> Google
>> > > > style.
>> > > > > I am not sure what is the difference between the, it would be
>> > > interesting
>> > > > > to research.
>> > > > >
>> > > > > What do you think about this?
>> > > > >
>> > > > > - Alex
>> > > > >
>> > > >
>> > >
>> >
>>
>
>
Re: Code style and checkstyle
Posted by Kalyan Kumar Kalvagadda <kk...@cloudera.com>.
Sasha,
I have downloaded the google style xml file and added it to intellij.
To change the existing code according to this style I should pick a file
and apply Code=>Format Code menu option.
Is there something else I should be doing?
-Kalyan
On Fri, Jul 7, 2017 at 4:34 PM, Alexander Kolbasov <ak...@cloudera.com>
wrote:
> I was able to use Google style in IntelliJ - the trick is to set checkstyle
> version to 7.1 and *apply* the change. After that you can point it to the
> google style xml file.
>
> Should we make this a requirement for new files?
>
> On Fri, Jul 7, 2017 at 6:06 PM, Sergio Pena <se...@cloudera.com>
> wrote:
>
> > I really like the google java style. Seems more updated to the developing
> > practices these days.
> >
> > On Fri, Jul 7, 2017 at 10:55 AM, Alexander Kolbasov <ak...@cloudera.com>
> > wrote:
> >
> > > Thanks Lina. What is AD_SAL and MD_SAL? Who were you citing as the
> person
> > > preferring yo use Google Java style? Is there any description of the
> > > differences between the two?
> > >
> > > - Alex
> > >
> > > On Fri, Jul 7, 2017 at 5:51 PM, Na Li <li...@cloudera.com> wrote:
> > >
> > > > Sasha,
> > > >
> > > > found the following online, and I think we can go for google style.
> > > >
> > > > "Sun coding style was used in original contribution of Controller and
> > > > AD-SAL and is mostly used in AD-SAL.
> > > >
> > > > Google Java Style is used in newer code such as YANGTools, MD-SAL,
> > etc...
> > > > so I would say prefer Google Java Style, since community decided on
> > going
> > > > with that style."
> > > >
> > > > I like the idea of enforcing style on new code. I can take a look on
> > how
> > > > easy to do that.
> > > >
> > > > Thanks,
> > > >
> > > > Lina
> > > >
> > > > On Fri, Jul 7, 2017 at 9:50 AM, Alexander Kolbasov <
> akolb@cloudera.com
> > >
> > > > wrote:
> > > >
> > > > > Hello,
> > > > >
> > > > > What would people thing about enforcing code style for Apache
> Sentry
> > > > code?
> > > > > There is a maven/IntelliJ plugin - checkstyle (
> > > > > http://checkstyle.sourceforge.net/index.html) that can help
> > enforcing
> > > > the
> > > > > style.
> > > > >
> > > > > The problem is that the current code isn't compliant and fails a
> lot
> > of
> > > > > checks and it wouldn't be reasonable to mass-modify a lot of
> existing
> > > > code
> > > > > to comply. Instead I would suggest that the new code and the code
> > that
> > > is
> > > > > modified extensively should comply and we can enforce this at code
> > > review
> > > > > time. It might be interesting to research whether checkstyle tool
> can
> > > be
> > > > > configured with a list of exclude/include files which can be
> modified
> > > as
> > > > > more files are made compliant.
> > > > >
> > > > > Another interesting issue is related to the style details. There
> are
> > > two
> > > > > styles supported out of the box - original Sun Java style and
> Google
> > > > style.
> > > > > I am not sure what is the difference between the, it would be
> > > interesting
> > > > > to research.
> > > > >
> > > > > What do you think about this?
> > > > >
> > > > > - Alex
> > > > >
> > > >
> > >
> >
>
Re: Code style and checkstyle
Posted by Alexander Kolbasov <ak...@cloudera.com>.
I was able to use Google style in IntelliJ - the trick is to set checkstyle
version to 7.1 and *apply* the change. After that you can point it to the
google style xml file.
Should we make this a requirement for new files?
On Fri, Jul 7, 2017 at 6:06 PM, Sergio Pena <se...@cloudera.com>
wrote:
> I really like the google java style. Seems more updated to the developing
> practices these days.
>
> On Fri, Jul 7, 2017 at 10:55 AM, Alexander Kolbasov <ak...@cloudera.com>
> wrote:
>
> > Thanks Lina. What is AD_SAL and MD_SAL? Who were you citing as the person
> > preferring yo use Google Java style? Is there any description of the
> > differences between the two?
> >
> > - Alex
> >
> > On Fri, Jul 7, 2017 at 5:51 PM, Na Li <li...@cloudera.com> wrote:
> >
> > > Sasha,
> > >
> > > found the following online, and I think we can go for google style.
> > >
> > > "Sun coding style was used in original contribution of Controller and
> > > AD-SAL and is mostly used in AD-SAL.
> > >
> > > Google Java Style is used in newer code such as YANGTools, MD-SAL,
> etc...
> > > so I would say prefer Google Java Style, since community decided on
> going
> > > with that style."
> > >
> > > I like the idea of enforcing style on new code. I can take a look on
> how
> > > easy to do that.
> > >
> > > Thanks,
> > >
> > > Lina
> > >
> > > On Fri, Jul 7, 2017 at 9:50 AM, Alexander Kolbasov <akolb@cloudera.com
> >
> > > wrote:
> > >
> > > > Hello,
> > > >
> > > > What would people thing about enforcing code style for Apache Sentry
> > > code?
> > > > There is a maven/IntelliJ plugin - checkstyle (
> > > > http://checkstyle.sourceforge.net/index.html) that can help
> enforcing
> > > the
> > > > style.
> > > >
> > > > The problem is that the current code isn't compliant and fails a lot
> of
> > > > checks and it wouldn't be reasonable to mass-modify a lot of existing
> > > code
> > > > to comply. Instead I would suggest that the new code and the code
> that
> > is
> > > > modified extensively should comply and we can enforce this at code
> > review
> > > > time. It might be interesting to research whether checkstyle tool can
> > be
> > > > configured with a list of exclude/include files which can be modified
> > as
> > > > more files are made compliant.
> > > >
> > > > Another interesting issue is related to the style details. There are
> > two
> > > > styles supported out of the box - original Sun Java style and Google
> > > style.
> > > > I am not sure what is the difference between the, it would be
> > interesting
> > > > to research.
> > > >
> > > > What do you think about this?
> > > >
> > > > - Alex
> > > >
> > >
> >
>
Re: Code style and checkstyle
Posted by Sergio Pena <se...@cloudera.com>.
I really like the google java style. Seems more updated to the developing
practices these days.
On Fri, Jul 7, 2017 at 10:55 AM, Alexander Kolbasov <ak...@cloudera.com>
wrote:
> Thanks Lina. What is AD_SAL and MD_SAL? Who were you citing as the person
> preferring yo use Google Java style? Is there any description of the
> differences between the two?
>
> - Alex
>
> On Fri, Jul 7, 2017 at 5:51 PM, Na Li <li...@cloudera.com> wrote:
>
> > Sasha,
> >
> > found the following online, and I think we can go for google style.
> >
> > "Sun coding style was used in original contribution of Controller and
> > AD-SAL and is mostly used in AD-SAL.
> >
> > Google Java Style is used in newer code such as YANGTools, MD-SAL, etc...
> > so I would say prefer Google Java Style, since community decided on going
> > with that style."
> >
> > I like the idea of enforcing style on new code. I can take a look on how
> > easy to do that.
> >
> > Thanks,
> >
> > Lina
> >
> > On Fri, Jul 7, 2017 at 9:50 AM, Alexander Kolbasov <ak...@cloudera.com>
> > wrote:
> >
> > > Hello,
> > >
> > > What would people thing about enforcing code style for Apache Sentry
> > code?
> > > There is a maven/IntelliJ plugin - checkstyle (
> > > http://checkstyle.sourceforge.net/index.html) that can help enforcing
> > the
> > > style.
> > >
> > > The problem is that the current code isn't compliant and fails a lot of
> > > checks and it wouldn't be reasonable to mass-modify a lot of existing
> > code
> > > to comply. Instead I would suggest that the new code and the code that
> is
> > > modified extensively should comply and we can enforce this at code
> review
> > > time. It might be interesting to research whether checkstyle tool can
> be
> > > configured with a list of exclude/include files which can be modified
> as
> > > more files are made compliant.
> > >
> > > Another interesting issue is related to the style details. There are
> two
> > > styles supported out of the box - original Sun Java style and Google
> > style.
> > > I am not sure what is the difference between the, it would be
> interesting
> > > to research.
> > >
> > > What do you think about this?
> > >
> > > - Alex
> > >
> >
>
Re: Code style and checkstyle
Posted by Na Li <li...@cloudera.com>.
Sasha,
The citing is from
https://ask.opendaylight.org/question/2377/odl-coding-style-sun-or-google-java/.
Some newer open source code is using google style. As mentioned in
https://www.quora.com/What-is-the-most-universal-Java-code-style-guidelines-and-why,
"I would say the Sun Coding Convention is the most widely used as it is the
oldest. However, I would consider using more recent coding conventions as
this is based on lessons learnt."
FYI:
OpenDaylight, the largest open source SDN controller.
AD_SAL is a flavor of the Service Abstraction Layer created for the
Controller project, whose task is primarily to create a layer against which
the applications can be developed without assuming the underlying SDN
protocol it's talking about. SDN is software defined network. It is related
to networking.
https://wiki.opendaylight.org/view/OpenDaylight_Controller:AD-SAL (this
page is deprecated)
Model-Driven SAL (MD-SAL) is a set of infrastructure services aimed at
providing common and generic support to application and plugin developers.
https://wiki.opendaylight.org/view/OpenDaylight_Controller:MD-SAL
On Fri, Jul 7, 2017 at 10:55 AM, Alexander Kolbasov <ak...@cloudera.com>
wrote:
> Thanks Lina. What is AD_SAL and MD_SAL? Who were you citing as the person
> preferring yo use Google Java style? Is there any description of the
> differences between the two?
>
> - Alex
>
> On Fri, Jul 7, 2017 at 5:51 PM, Na Li <li...@cloudera.com> wrote:
>
> > Sasha,
> >
> > found the following online, and I think we can go for google style.
> >
> > "Sun coding style was used in original contribution of Controller and
> > AD-SAL and is mostly used in AD-SAL.
> >
> > Google Java Style is used in newer code such as YANGTools, MD-SAL, etc...
> > so I would say prefer Google Java Style, since community decided on going
> > with that style."
> >
> > I like the idea of enforcing style on new code. I can take a look on how
> > easy to do that.
> >
> > Thanks,
> >
> > Lina
> >
> > On Fri, Jul 7, 2017 at 9:50 AM, Alexander Kolbasov <ak...@cloudera.com>
> > wrote:
> >
> > > Hello,
> > >
> > > What would people thing about enforcing code style for Apache Sentry
> > code?
> > > There is a maven/IntelliJ plugin - checkstyle (
> > > http://checkstyle.sourceforge.net/index.html) that can help enforcing
> > the
> > > style.
> > >
> > > The problem is that the current code isn't compliant and fails a lot of
> > > checks and it wouldn't be reasonable to mass-modify a lot of existing
> > code
> > > to comply. Instead I would suggest that the new code and the code that
> is
> > > modified extensively should comply and we can enforce this at code
> review
> > > time. It might be interesting to research whether checkstyle tool can
> be
> > > configured with a list of exclude/include files which can be modified
> as
> > > more files are made compliant.
> > >
> > > Another interesting issue is related to the style details. There are
> two
> > > styles supported out of the box - original Sun Java style and Google
> > style.
> > > I am not sure what is the difference between the, it would be
> interesting
> > > to research.
> > >
> > > What do you think about this?
> > >
> > > - Alex
> > >
> >
>
Re: Code style and checkstyle
Posted by Alexander Kolbasov <ak...@cloudera.com>.
Thanks Lina. What is AD_SAL and MD_SAL? Who were you citing as the person
preferring yo use Google Java style? Is there any description of the
differences between the two?
- Alex
On Fri, Jul 7, 2017 at 5:51 PM, Na Li <li...@cloudera.com> wrote:
> Sasha,
>
> found the following online, and I think we can go for google style.
>
> "Sun coding style was used in original contribution of Controller and
> AD-SAL and is mostly used in AD-SAL.
>
> Google Java Style is used in newer code such as YANGTools, MD-SAL, etc...
> so I would say prefer Google Java Style, since community decided on going
> with that style."
>
> I like the idea of enforcing style on new code. I can take a look on how
> easy to do that.
>
> Thanks,
>
> Lina
>
> On Fri, Jul 7, 2017 at 9:50 AM, Alexander Kolbasov <ak...@cloudera.com>
> wrote:
>
> > Hello,
> >
> > What would people thing about enforcing code style for Apache Sentry
> code?
> > There is a maven/IntelliJ plugin - checkstyle (
> > http://checkstyle.sourceforge.net/index.html) that can help enforcing
> the
> > style.
> >
> > The problem is that the current code isn't compliant and fails a lot of
> > checks and it wouldn't be reasonable to mass-modify a lot of existing
> code
> > to comply. Instead I would suggest that the new code and the code that is
> > modified extensively should comply and we can enforce this at code review
> > time. It might be interesting to research whether checkstyle tool can be
> > configured with a list of exclude/include files which can be modified as
> > more files are made compliant.
> >
> > Another interesting issue is related to the style details. There are two
> > styles supported out of the box - original Sun Java style and Google
> style.
> > I am not sure what is the difference between the, it would be interesting
> > to research.
> >
> > What do you think about this?
> >
> > - Alex
> >
>
Re: Code style and checkstyle
Posted by Na Li <li...@cloudera.com>.
Sasha,
found the following online, and I think we can go for google style.
"Sun coding style was used in original contribution of Controller and
AD-SAL and is mostly used in AD-SAL.
Google Java Style is used in newer code such as YANGTools, MD-SAL, etc...
so I would say prefer Google Java Style, since community decided on going
with that style."
I like the idea of enforcing style on new code. I can take a look on how
easy to do that.
Thanks,
Lina
On Fri, Jul 7, 2017 at 9:50 AM, Alexander Kolbasov <ak...@cloudera.com>
wrote:
> Hello,
>
> What would people thing about enforcing code style for Apache Sentry code?
> There is a maven/IntelliJ plugin - checkstyle (
> http://checkstyle.sourceforge.net/index.html) that can help enforcing the
> style.
>
> The problem is that the current code isn't compliant and fails a lot of
> checks and it wouldn't be reasonable to mass-modify a lot of existing code
> to comply. Instead I would suggest that the new code and the code that is
> modified extensively should comply and we can enforce this at code review
> time. It might be interesting to research whether checkstyle tool can be
> configured with a list of exclude/include files which can be modified as
> more files are made compliant.
>
> Another interesting issue is related to the style details. There are two
> styles supported out of the box - original Sun Java style and Google style.
> I am not sure what is the difference between the, it would be interesting
> to research.
>
> What do you think about this?
>
> - Alex
>