flink-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Till Rohrmann <trohrm...@apache.org>
Subject Re: [DISCUSS] Java code style
Date Mon, 26 Oct 2015 17:42:23 GMT
Concerning question 2 Tabs vs. Spaces, in case of spaces we would have to
decide on the number of spaces, too. The Google Java style says to use a 2
space indentation, which is in my opinion sufficient to distinguish
different indentations levels from each other. Furthermore, it would save
some space.

I would not vote -1 if we keep tabs.



On Sat, Oct 24, 2015 at 8:33 PM, Henry Saputra <henry.saputra@gmail.com>
wrote:

> +1 for adding restriction for Javadoc at least at the header of public
> classes and methods.
>
> We did the exercise in Twill and seemed to work pretty well.
>
> On Fri, Oct 23, 2015 at 1:34 AM, Maximilian Michels <mxm@apache.org>
> wrote:
> > I don't think lazily adding comments will work. However, I'm fine with
> > adding all the checkstyle rules one module at a time (with a jira
> > issue to keep track of the modules already converted). It's not going
> > to happen that we lazily add comments because that's the reason why
> > comments are missing in the first place...
> >
> > On Fri, Oct 23, 2015 at 12:05 AM, Henry Saputra <henry.saputra@gmail.com>
> wrote:
> >> Could we make certain rules to give warning instead of error?
> >>
> >> This would allow us to cherry-pick certain rules we would like people
> >> to follow but not strictly enforced.
> >>
> >> - Henry
> >>
> >> On Thu, Oct 22, 2015 at 9:20 AM, Stephan Ewen <sewen@apache.org> wrote:
> >>> I don't think a "let add comments to everything" effort gives us good
> >>> comments, actually. It just gives us checkmark comments that make the
> rules
> >>> pass.
> >>>
> >>> On Thu, Oct 22, 2015 at 3:29 PM, Fabian Hueske <fhueske@gmail.com>
> wrote:
> >>>
> >>>> Sure, I don't expect it to be free.
> >>>> But everybody should be aware of the cost of adding this code style,
> i.e.,
> >>>> spending a huge amount of time on reformatting and documenting code.
> >>>>
> >>>> Alternatively, we could drop the JavaDocs rule and make the transition
> >>>> significantly cheaper.
> >>>>
> >>>> 2015-10-22 15:24 GMT+02:00 Till Rohrmann <trohrmann@apache.org>:
> >>>>
> >>>> > There ain’t no such thing as a free lunch and code style.
> >>>> >
> >>>> > On Thu, Oct 22, 2015 at 3:13 PM, Maximilian Michels <mxm@apache.org
> >
> >>>> > wrote:
> >>>> >
> >>>> > > I think we have to document all these classes. Code Style
doesn't
> come
> >>>> > > for free :)
> >>>> > >
> >>>> > > On Thu, Oct 22, 2015 at 3:09 PM, Fabian Hueske <fhueske@gmail.com
> >
> >>>> > wrote:
> >>>> > > > Any ideas how to deal with the mandatory JavaDoc rule
for
> existing
> >>>> > code?
> >>>> > > > Just adding empty headers to make the checkstyle pass
or start a
> >>>> > serious
> >>>> > > > effort to add the missing docs?
> >>>> > > >
> >>>> > > > 2015-10-21 13:31 GMT+02:00 Matthias J. Sax <mjsax@apache.org>:
> >>>> > > >
> >>>> > > >> Agreed. That's the reason why I am in favor of using
vanilla
> Google
> >>>> > code
> >>>> > > >> style.
> >>>> > > >>
> >>>> > > >> On 10/21/2015 12:31 PM, Stephan Ewen wrote:
> >>>> > > >> > We started out originally with mixed tab/spaces,
but it
> ended up
> >>>> > with
> >>>> > > >> > people mixing spaces and tabs arbitrarily, and
there is
> little way
> >>>> > to
> >>>> > > >> > enforce Matthias' specific suggestion via checkstyle.
> >>>> > > >> > That's why we dropped spaces alltogether...
> >>>> > > >> >
> >>>> > > >> > On Wed, Oct 21, 2015 at 12:03 PM, Gyula Fóra
<
> >>>> gyula.fora@gmail.com>
> >>>> > > >> wrote:
> >>>> > > >> >
> >>>> > > >> >> I think the nice thing about a common codestyle
is that
> everyone
> >>>> > can
> >>>> > > set
> >>>> > > >> >> the template in the IDE and use the formatting
commands.
> >>>> > > >> >>
> >>>> > > >> >> Matthias's suggestion makes this practically
impossible so
> -1 for
> >>>> > > mixed
> >>>> > > >> >> tabs/spaces from my side.
> >>>> > > >> >>
> >>>> > > >> >> Matthias J. Sax <mjsax@apache.org>
ezt írta (időpont:
> 2015. okt.
> >>>> > > 21.,
> >>>> > > >> Sze,
> >>>> > > >> >> 11:46):
> >>>> > > >> >>
> >>>> > > >> >>> I actually like tabs a lot, however,
in a "mixed" style
> together
> >>>> > > with
> >>>> > > >> >>> spaces. Example:
> >>>> > > >> >>>
> >>>> > > >> >>>         myVar.callMethod(param1, //
many more
> >>>> > > >> >>>         .................paramX); //
the dots mark space
> >>>> indention
> >>>> > > >> >>>
> >>>> > > >> >>> indenting "paramX" with tabs does not
give nice aliment.
> Not
> >>>> sure
> >>>> > if
> >>>> > > >> >>> this would be a feasible compromise
to keeps tabs in
> general,
> >>>> but
> >>>> > > use
> >>>> > > >> >>> space for cases as above.
> >>>> > > >> >>>
> >>>> > > >> >>> If this in no feasible compromise, I
would prefer space to
> get
> >>>> the
> >>>> > > >> >>> correct indention in examples as above.
Even if this
> result in a
> >>>> > > >> >>> complete reformatting of the whole code.
> >>>> > > >> >>>
> >>>> > > >> >>>
> >>>> > > >> >>> Why this? Everybody can set this in
it's IDE/editor as
> he/she
> >>>> > > wishes...
> >>>> > > >> >>>
> >>>> > > >> >>>>> If we keep tabs, we will have
to specify the line length
> >>>> > relative
> >>>> > > to
> >>>> > > >> a
> >>>> > > >> >>> tab
> >>>> > > >> >>>>> size (like 4).
> >>>> > > >> >>>
> >>>> > > >> >>>
> >>>> > > >> >>> -Matthias
> >>>> > > >> >>>
> >>>> > > >> >>>
> >>>> > > >> >>>
> >>>> > > >> >>>
> >>>> > > >> >>>
> >>>> > > >> >>> On 10/21/2015 11:06 AM, Ufuk Celebi
wrote:
> >>>> > > >> >>>> To summarize up to this point:
> >>>> > > >> >>>>
> >>>> > > >> >>>> - All are in favour of Google check
style (with the
> following
> >>>> > > possible
> >>>> > > >> >>>> exceptions)
> >>>> > > >> >>>> - Proposed exceptions so far:
> >>>> > > >> >>>>   * Specific line length 100 vs.
120 characters
> >>>> > > >> >>>>   * Keep tabs instead converting
to spaces (this would
> >>>> translate
> >>>> > to
> >>>> > > >> >>>> skipping/coming up with some indentation
rules as well)
> >>>> > > >> >>>>
> >>>> > > >> >>>> If we keep tabs, we will have to
specify the line length
> >>>> relative
> >>>> > > to a
> >>>> > > >> >>> tab
> >>>> > > >> >>>> size (like 4).
> >>>> > > >> >>>>
> >>>> > > >> >>>> Let’s keep the discussion going
a little longer. I think
> it has
> >>>> > > >> >> proceeded
> >>>> > > >> >>>> in a very reasonable manner so far.
Thanks for this!
> >>>> > > >> >>>>
> >>>> > > >> >>>> – Ufuk
> >>>> > > >> >>>>
> >>>> > > >> >>>> On Wed, Oct 21, 2015 at 10:29 AM,
Fabian Hueske <
> >>>> > fhueske@gmail.com
> >>>> > > >
> >>>> > > >> >>> wrote:
> >>>> > > >> >>>>
> >>>> > > >> >>>>> Thanks Max for checking the
modifications by the Google
> code
> >>>> > > style.
> >>>> > > >> >>>>> It is very good to know, that
the impact on the code base
> >>>> would
> >>>> > > not
> >>>> > > >> be
> >>>> > > >> >>> too
> >>>> > > >> >>>>> massive. If the Google code
style would have touched
> almost
> >>>> > every
> >>>> > > >> >> line,
> >>>> > > >> >>> I
> >>>> > > >> >>>>> would have been in favor of
converting to spaces.
> However,
> >>>> your
> >>>> > > >> >>> assessment
> >>>> > > >> >>>>> is a strong argument to continue
with tabs, IMO.
> >>>> > > >> >>>>>
> >>>> > > >> >>>>> Regarding the line length limit,
I personally find 100
> chars
> >>>> too
> >>>> > > >> >> narrow
> >>>> > > >> >>> but
> >>>> > > >> >>>>> would be +1 for having a limit.
> >>>> > > >> >>>>>
> >>>> > > >> >>>>> +1 for discussing the Scala
style in a separate thread.
> >>>> > > >> >>>>>
> >>>> > > >> >>>>> Fabian
> >>>> > > >> >>>>>
> >>>> > > >> >>>>> 2015-10-20 18:12 GMT+02:00 Maximilian
Michels <
> mxm@apache.org
> >>>> >:
> >>>> > > >> >>>>>
> >>>> > > >> >>>>>> I'm a little less excited
about this. You might not be
> aware
> >>>> > but,
> >>>> > > >> for
> >>>> > > >> >>>>>> a large portion of the source
code, we already follow
> the
> >>>> > Google
> >>>> > > >> >> style
> >>>> > > >> >>>>>> guide. The main changes
will be tabs->spaces and 80/100
> >>>> > > characters
> >>>> > > >> >>>>>> line limit.
> >>>> > > >> >>>>>>
> >>>> > > >> >>>>>> Out of curiosity, I ran
the official Google Style
> Checkstyle
> >>>> > > >> >>>>>> configuration to confirm
my suspicion:
> >>>> > > >> >>>>>>
> >>>> > > >> >>>>>>
> >>>> > > >> >>>>>
> >>>> > > >> >>>
> >>>> > > >> >>
> >>>> > > >>
> >>>> > >
> >>>> >
> >>>>
> https://github.com/checkstyle/checkstyle/blob/master/src/main/resources/google_checks.xml
> >>>> > > >> >>>>>> The changes are very little
if we turn off line length
> limit
> >>>> > and
> >>>> > > >> >>>>>> tabs-to-spaces conversion.
> >>>> > > >> >>>>>>
> >>>> > > >> >>>>>> There are some things I
really like about the Google
> style,
> >>>> > e.g.
> >>>> > > >> >> every
> >>>> > > >> >>>>>> class has to have a JavaDoc
and spaces after keywords
> (can't
> >>>> > > stand
> >>>> > > >> if
> >>>> > > >> >>>>>> there aren't any). I'm not
sure if we should change
> tabs to
> >>>> > > spaces,
> >>>> > > >> >>>>>> because it means touching
almost every single line of
> code.
> >>>> > > However,
> >>>> > > >> >>>>>> if we keep the tabs, we
cannot make use of the different
> >>>> > > indention
> >>>> > > >> >> for
> >>>> > > >> >>>>>> case statements or wrapped
lines...maybe that's a
> compromise
> >>>> we
> >>>> > > can
> >>>> > > >> >>>>>> live with.
> >>>> > > >> >>>>>>
> >>>> > > >> >>>>>> If we introduce the Google
Style for Java, will we also
> >>>> impose
> >>>> > a
> >>>> > > >> >>>>>> stricter style check for
Scala? IMHO the line length is
> the
> >>>> > > >> strictest
> >>>> > > >> >>>>>> part of the Scala Checkstyle.
> >>>> > > >> >>>>>>
> >>>> > > >> >>>>>>
> >>>> > > >> >>>>>> On Tue, Oct 20, 2015 at
4:14 PM, Henry Saputra <
> >>>> > > >> >>> henry.saputra@gmail.com>
> >>>> > > >> >>>>>> wrote:
> >>>> > > >> >>>>>>> 1) yes. Been dancing
this issue for a while. Let's
> pull the
> >>>> > > >> trigger.
> >>>> > > >> >>>>> Did
> >>>> > > >> >>>>>>> the exercise with Tachyon
while back and did help
> >>>> readability
> >>>> > > and
> >>>> > > >> >>>>>>> homogeneity of code.
> >>>> > > >> >>>>>>>
> >>>> > > >> >>>>>>> 2) +1 for Google Java
style with documented exceptions
> and
> >>>> > > >> >> explanation
> >>>> > > >> >>>>> on
> >>>> > > >> >>>>>>> why.
> >>>> > > >> >>>>>>>
> >>>> > > >> >>>>>>> On Tuesday, October
20, 2015, Ufuk Celebi <
> uce@apache.org>
> >>>> > > wrote:
> >>>> > > >> >>>>>>>
> >>>> > > >> >>>>>>>> DISCLAIMER: This
is not my personal idea, but a
> community
> >>>> > > >> >> discussion
> >>>> > > >> >>>>>> from
> >>>> > > >> >>>>>>>> some time ago. Don't
kill the messenger.
> >>>> > > >> >>>>>>>>
> >>>> > > >> >>>>>>>> In March we were
discussing issues with heterogeneity
> of
> >>>> the
> >>>> > > code
> >>>> > > >> >>> [1].
> >>>> > > >> >>>>>> The
> >>>> > > >> >>>>>>>> summary is that
we had a consensus to enforce a
> stricter
> >>>> code
> >>>> > > >> style
> >>>> > > >> >>> on
> >>>> > > >> >>>>>> our
> >>>> > > >> >>>>>>>> Java code base in
order to make it easier to switch
> between
> >>>> > > >> >> projects
> >>>> > > >> >>>>>> and to
> >>>> > > >> >>>>>>>> have clear rules
for new contributions. The main
> proposal
> >>>> in
> >>>> > > the
> >>>> > > >> >> last
> >>>> > > >> >>>>>>>> discussion was to
go with Google's Java code style.
> Not all
> >>>> > > were
> >>>> > > >> >>> fully
> >>>> > > >> >>>>>>>> satisfied with this,
but still everyone agreed on
> some kind
> >>>> > of
> >>>> > > >> >> style.
> >>>> > > >> >>>>>>>>
> >>>> > > >> >>>>>>>> I think the upcoming
0.10 release is a good point to
> >>>> finally
> >>>> > go
> >>>> > > >> >>>>> through
> >>>> > > >> >>>>>>>> with these changes
(right after the
> release/branch-off).
> >>>> > > >> >>>>>>>>
> >>>> > > >> >>>>>>>> I propose to go
with Google's Java code style [2] as
> >>>> proposed
> >>>> > > >> >>> earlier.
> >>>> > > >> >>>>>>>>
> >>>> > > >> >>>>>>>> PROs:
> >>>> > > >> >>>>>>>> - Clear style guide
available
> >>>> > > >> >>>>>>>> - Tooling like checkstyle
rules, IDE plugins already
> >>>> > available
> >>>> > > >> >>>>>>>>
> >>>> > > >> >>>>>>>> CONs:
> >>>> > > >> >>>>>>>> - Fully breaks our
current style
> >>>> > > >> >>>>>>>>
> >>>> > > >> >>>>>>>> The main problem
with this will be open pull requests,
> >>>> which
> >>>> > > will
> >>>> > > >> >> be
> >>>> > > >> >>>>>> harder
> >>>> > > >> >>>>>>>> to merge after all
the changes. On the other hand,
> should
> >>>> > pull
> >>>> > > >> >>>>> requests
> >>>> > > >> >>>>>>>> that have been open
for a long time block this? Most
> of the
> >>>> > > >> >> important
> >>>> > > >> >>>>>>>> changes will be
merged for the release anyways. I
> think in
> >>>> > the
> >>>> > > >> long
> >>>> > > >> >>>>> run
> >>>> > > >> >>>>>> we
> >>>> > > >> >>>>>>>> will gain more than
we loose by this (more homogenous
> code,
> >>>> > > clear
> >>>> > > >> >>>>>> rules).
> >>>> > > >> >>>>>>>> And it is questionable
whether we will ever be able
> to do
> >>>> > such
> >>>> > > a
> >>>> > > >> >>>>> change
> >>>> > > >> >>>>>> in
> >>>> > > >> >>>>>>>> the future if we
cannot do it now. The project will
> most
> >>>> > likely
> >>>> > > >> >> grow
> >>>> > > >> >>>>> and
> >>>> > > >> >>>>>>>> attract more contributors,
at which point it will be
> even
> >>>> > > harder
> >>>> > > >> to
> >>>> > > >> >>>>> do.
> >>>> > > >> >>>>>>>>
> >>>> > > >> >>>>>>>> Please make sure
to answer the following points in the
> >>>> > > discussion:
> >>>> > > >> >>>>>>>>
> >>>> > > >> >>>>>>>> 1) Are you (still)
in favour of enforcing stricter
> rules on
> >>>> > the
> >>>> > > >> >> Java
> >>>> > > >> >>>>>>>> codebase?
> >>>> > > >> >>>>>>>>
> >>>> > > >> >>>>>>>> 2) If yes, would
you be OK with the Google's Java code
> >>>> style?
> >>>> > > >> >>>>>>>>
> >>>> > > >> >>>>>>>> – Ufuk
> >>>> > > >> >>>>>>>>
> >>>> > > >> >>>>>>>> [1]
> >>>> > > >> >>>>>>>>
> >>>> > > >> >>>>>>>>
> >>>> > > >> >>>>>>
> >>>> > > >> >>>>>
> >>>> > > >> >>>
> >>>> > > >> >>
> >>>> > > >>
> >>>> > >
> >>>> >
> >>>>
> http://mail-archives.apache.org/mod_mbox/flink-dev/201503.mbox/%3cCANC1h_voN0B5omNWZxcHTyzwHAKeGhbZVQUyK7S9o2A36b891Q@mail.gmail.com%3e
> >>>> > > >> >>>>>>>>
> >>>> > > >> >>>>>>>> [2]
> https://google.github.io/styleguide/javaguide.html
> >>>> > > >> >>>>>>>>
> >>>> > > >> >>>>>>
> >>>> > > >> >>>>>
> >>>> > > >> >>>>
> >>>> > > >> >>>
> >>>> > > >> >>>
> >>>> > > >> >>
> >>>> > > >> >
> >>>> > > >>
> >>>> > > >>
> >>>> > >
> >>>> >
> >>>>
>

Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message