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 <se...@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 <fhue...@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 <trohrm...@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 <m...@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 <fhue...@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 <mj...@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.f...@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 <mj...@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 < >> > fhue...@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 <m...@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.sapu...@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 <u...@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_von0b5omnwzxchtyzwhakeghbzvquyk7s9o2a36b8...@mail.gmail.com%3e >> > > >> >>>>>>>> >> > > >> >>>>>>>> [2] https://google.github.io/styleguide/javaguide.html >> > > >> >>>>>>>> >> > > >> >>>>>> >> > > >> >>>>> >> > > >> >>>> >> > > >> >>> >> > > >> >>> >> > > >> >> >> > > >> > >> > > >> >> > > >> >> > > >> > >>
