We have discussed this issue several times in the past and never got around to do it.
Although I agree that it would be nice to have a stricter code style, I don't think we should introduce a code style. IMO, at this point the costs (losing the history, PR hassle, etc.) would be too high compared to the benefits (that I am aware of). I agree with Till that encouraging a code style without enforcing it does not make a lot of sense. If we enforce it, we need to touch all files and PRs. On the other hand, it's not clear to me how much we would gain with a code style and what's the cost of not having one. Best, Fabian 2017-02-23 17:52 GMT+01:00 Jinkui Shi <shijinkui...@163.com>: > Thanks to discuss this problem again. > 1. Google checkstyle is good for java. > 2. scala check style is here [1] > 3. We can make a Flink plan contain issues, one sub-issue one rule. > Resolve this in short time. > > Current code style may be historical accumulate. If we don’t normalize the > code step by step, new code will also follow bad style without strict > checking. > We can adjust the exist code once one rule. But there will be lots of > conflict in the not merged pull requests. Need all the pull request resolve > the conflict manually. > Resolve long term trouble in short time:) > > > [1] http://www.scalastyle.org/ <http://www.scalastyle.org/> > > On Feb 23, 2017, at 23:31, Aljoscha Krettek <aljos...@apache.org> wrote: > > > > 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.da...@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 <ches...@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.da...@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 <ches...@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 <u...@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 <ykt...@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.da...@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. > >>>>>>>>>> > >>>>>>>>>> > >>>>>>>>>> > >>> > >> > >
