flink-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Aljoscha Krettek <aljos...@apache.org>
Subject Re: [DISCUSS] Code style / checkstyle
Date Thu, 23 Feb 2017 15:31:03 GMT
If we go for a codestyle/checkstyle I would suggest to use the Google
style. This already has checkstyle, IntelliJ style, Eclipse style and a
code formatting tool and is well established. However, some people will not
like this style. In general, I think we will never manage to find a style
that all people will like.

On Wed, 22 Feb 2017 at 18:36 Dawid Wysakowicz <wysakowicz.dawid@gmail.com>
wrote:

> So how about preparing a code style and corresponding checkstyle and
> enabling it gradually module by module whenever shepherd/commiter with
> expertise in a module will have time to introduce/check such a change?
> Maybe it will make the "snowball effect" happen?
> I agree there is no point in preparing code style/checkstyle until it will
> be introduced somewhere. I will be willing to work on the checkstyle if
> some volunteering modules appear.
>
> 2017-02-22 17:09 GMT+01:00 Chesnay Schepler <chesnay@apache.org>:
>
> > For file where we don't enforce checkstyle things should work they way
> > they do right now.
> >
> > Turn off auto-formatting, and only format code that you touched and
> that's
> > it. For these
> > modification we will have to check them manually in the PRs as we do now.
> >
> >
> > On 22.02.2017 16:22, Greg Hogan wrote:
> >
> >> Will not the code style be applied on save to any user-modified file? So
> >> this will clutter PRs and overwrite history.
> >>
> >> On Wed, Feb 22, 2017 at 6:19 AM, Dawid Wysakowicz <
> >> wysakowicz.dawid@gmail.com> wrote:
> >>
> >> I also agree with Till and Chesnayl. Anyway as to "capture the current
> >>> style" I have some doubts if this is possible, as it changes file to
> >>> file.
> >>>
> >>> Chesnay's suggestion as to were enforce the checkstyle seems reasonable
> >>> to
> >>> me, but I am quite new to the community :).
> >>> Enabling checkstyle for particular packages is possible.
> >>>
> >>> 2017-02-22 12:07 GMT+01:00 Chesnay Schepler <chesnay@apache.org>:
> >>>
> >>> I agree with Till.
> >>>>
> >>>> I would propose enforcing checkstyle on a subset of the modules,
> >>>>
> >>> basically
> >>>
> >>>> those that are not
> >>>> flink-runtime, flink-java, flink-streaming-java. These are the ones
> imo
> >>>> where messing with the history
> >>>> can be detrimental; for the others it isn't really important imo.
> >>>> (Note that i excluded scala code since i don't know the state of
> >>>> checkstyle compliance there)
> >>>>
> >>>> For flink-runtime we could maybe (don't know if it is supported)
> enforce
> >>>> checkstyle for all classes
> >>>> under o.a.f.migration, so that at least the flip-6 code is covered.
> >>>>
> >>>> Similarly, enforcing checkstyle for all tests should be fine as well.
> >>>>
> >>>> Regards,
> >>>> Chesnay
> >>>>
> >>>>
> >>>> On 22.02.2017 11:48, Till Rohrmann wrote:
> >>>>
> >>>> I think that not enforcing a code style is as good as not having any
> >>>>>
> >>>> code
> >>>
> >>>> style to be honest. Having an IntelliJ or Eclipse profile is nice and
> >>>>>
> >>>> some
> >>>
> >>>> people will probably use it, but my gut feeling is that the majority
> >>>>>
> >>>> won't
> >>>
> >>>> notice it.
> >>>>>
> >>>>> Cheers,
> >>>>> Till
> >>>>>
> >>>>> On Wed, Feb 22, 2017 at 11:15 AM, Ufuk Celebi <uce@apache.org>
> wrote:
> >>>>>
> >>>>> Kurt's proposal sounds reasonable.
> >>>>>
> >>>>>> What about the following:
> >>>>>> - We try to capture the current style in an code style configuration
> >>>>>> (for IntelliJ and maybe Eclipse)
> >>>>>> - We provide that on the website for contributors to download
> >>>>>> - We don't enforce it, but new contributions and changes are
free to
> >>>>>> format with this style as changes happen
> >>>>>>
> >>>>>> Practically speaking, this should not change much except maybe
the
> >>>>>> import order or whitespace after certain keywords, etc.
> >>>>>>
> >>>>>>
> >>>>>> On Wed, Feb 22, 2017 at 4:48 AM, Kurt Young <ykt836@gmail.com>
> wrote:
> >>>>>>
> >>>>>> +1 to provide a unified code style for both java and scala.
> >>>>>>>
> >>>>>>> -1 to adjust all the existing code to the new style in one
step.
> The
> >>>>>>>
> >>>>>>> code's
> >>>>>>
> >>>>>> history contains very helpful information which can help
> >>>>>>> develops to understand why these codes are added, which
jira is
> >>>>>>>
> >>>>>> related.
> >>>
> >>>> This information is too valuable to lose. I think we can
> >>>>>>> do the reformat thing step by step, each time when the codes
being
> >>>>>>>
> >>>>>>> changed,
> >>>>>>
> >>>>>> we can adopt them to the new style.
> >>>>>>> IMHO this is also the reason why the unified code style
is
> important.
> >>>>>>>
> >>>>>>>
> >>>>>>> Best,
> >>>>>>> Kurt
> >>>>>>>
> >>>>>>> On Wed, Feb 22, 2017 at 5:50 AM, Dawid Wysakowicz <
> >>>>>>> wysakowicz.dawid@gmail.com> wrote:
> >>>>>>>
> >>>>>>> Hi,
> >>>>>>>
> >>>>>>>> I would like to resurrect the discussing ([1]
> >>>>>>>> <http://apache-flink-mailing-list-archive.1008284.n3.
> >>>>>>>> nabble.com/Code-style-guideline-for-Scala-td7526.html>
> >>>>>>>> , [2]
> >>>>>>>> <http://apache-flink-mailing-list-archive.1008284.n3.
> >>>>>>>> nabble.com/Intellij-code-style-td11092.html>)
> >>>>>>>> about creating unified code style(that could be imported
to at
> least
> >>>>>>>> IntelliJ and Eclipse) and corresponding stricter checkstyle
rules.
> >>>>>>>>
> >>>>>>>> I know that the hardest part is to adjust the existing
code to the
> >>>>>>>>
> >>>>>>> new
> >>>
> >>>> checkstyle rules. Do you believe it would be worth the effort? All
> >>>>>>>> suggestions are welcome.
> >>>>>>>>
> >>>>>>>>
> >>>>>>>>
> >
>

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