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 Thu, 22 Oct 2015 13:24:59 GMT
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