I like the idea of a recommended reading list for Geode contributors. My concerns around adopting broad standards and guidelines that can’t be automatically checked & applied are twofold:
a) what is the policy regarding existing code? Is every PR going forward expected to bring every file it touches into conformance? b) how do we avoid PR reviews devolving into style-nitpicking? If a PR clearly fixes a bug, but someone doesn’t like a variable name, would that be reason to give it a -1? Even better would be leading by example [if we can’t overhaul the entire codebase, maybe at least be able to point to one file that really embodies our desired coding standards]. People over process. Code is either readable or it isn’t. If you are reviewing a PR and the proposed changes are difficult to follow, a simple and kind explanation is probably more constructive than citing the offender with the chapter and section of their violation. > On Jun 24, 2019, at 3:31 PM, Alexander Murmann <amurm...@apache.org> wrote: > >> Is there an entry in the Coding Standard's Rules section that you feel is > irrelevant or incorrect? Please pick an example with a link to it so we can > discuss it. > > I haven't seen any rules in there that I think are irrelevant or incorrect. > My reasoning is a little different from that: > I think there are two different, competing goals we can optimize for: > a) The rules should be as complete as possible. > b) New contributors should be able to quickly catch up to what the coding > standard is for this project. > > I'd rather optimize for a) over b). To that end I'd prefer to leave > absolutely valid, but arguably obvious rules like MET02-J. Do not use > deprecated or obsolete classes or methods > <https://wiki.sei.cmu.edu/confluence/display/java/MET02-J.+Do+not+use+deprecated+or+obsolete+classes+or+methods> > or IDS00-J. Prevent SQL injection > <https://wiki.sei.cmu.edu/confluence/display/java/IDS00-J.+Prevent+SQL+injection> > off > and instead highlight the ways we are more opinionated then some other Java > projects (e.g. "Avoid introducing new statics", "limit method length", > "don't use abbreviated names") > > We could also go down the path of a compromise and highlight the rules we > care most about that will allow someone who is already a competent > developer to get up to speed quickly on our project and refer to the SEI > CERT rules as a recommended read for developers who are newer to Java. The > difference to your proposal would be that the Geode wiki page would > highlight what's most important and potentially surprising rather than be > an addendum to the already very large corpus of SEI CERT rules. > > > On Mon, Jun 24, 2019 at 3:15 PM Kirk Lund <kl...@apache.org> wrote: > >> Java is complicated and Apache Geode is complicated, hence it's a large >> Coding Standard. *Effective Java* is similarly *large* if you compare it to >> the *Google Java Style Guide*. >> >> The "*rules*" are "*guidelines*" -- I think you're being too literal. Also, >> please remember what I said: >> >> I'm not proposing we rigidly and blindly follow this Coding Standard. We >>> can extend or even supersede portions of the adopted Coding Standard with >>> our own Addendum. The Coding Standard Addendum would exist on the Apache >>> Geode Wiki to define Geode-specific rules or recommendations. What I'd >> like >>> to see happen is for us to use the SEI CERT Coding Standard for Java as a >>> starting point for our own Coding Standard. The resulting Coding Standard >>> for Geode can be as static or as living and evolving as we wish. >>> >> >> Is there an entry in the Coding Standard's Rules section that you feel is >> irrelevant or incorrect? Please pick an example with a link to it so we can >> discuss it. >> >> PS: There aren't any other Coding Standards that I would personally write >> or recommend. >> >> On Mon, Jun 24, 2019 at 2:50 PM Alexander Murmann <amurm...@apache.org> >> wrote: >> >>> Hi Kirk, >>> >>> I think having a coding standard that goes beyond a formatting style >> guide >>> is a great idea. There are many interesting things in the SEI CERT >>> standard. However, it's also massive. I see 13 rules just about methods. >>> Yet some guidelines that would be most important to me like limiting >> method >>> length and number of parameters are missing. >>> >>> I wonder if we might be better off taking the rules we like from SEI >> CERT, >>> adding our own and aiming for a much smaller set of guidelines. I'd hope >>> for something like a one-pager. If it gets much longer than that, it >>> becomes burdensome to read for newcomers and we want to make sure they >> can >>> quickly take in what's most important. >>> >>> I also prefer "guidelines" over "rules". I'd like to have a discussion if >>> someone is not following a guideline, rather than creating the impression >>> that all rules must be followed, no matter what the circumstances are. >>> >>> >>> On Mon, Jun 24, 2019 at 2:16 PM Kirk Lund <kl...@apache.org> wrote: >>> >>>> Apache Geode has a Code Style Guide [1] which is currently defined as >>>> following the Google Java Style Guide [2]. This style guide is a good >>>> starting point, but it deals primarily with formatting of code and is a >>>> fairly dated and static document that doesn't evolve much. >>>> >>>> I'd like to propose that the Geode dev community adopt a Coding >> Standard >>> in >>>> addition to the Style Guide. Specifically, I believe that having our >>>> community follow the SEI CERT Coding Standard [3] for Java [4] would >>>> benefit us greatly. There are also Coding Standards for C and C++ that >> we >>>> could consider if we decide to use the one for Java. >>>> >>>> SEI CERT Coding Standards are completely documented on their wiki which >>> is >>>> open to having anyone join and become involved in their community. They >>> are >>>> also available in book form (including on amazon.com). >>>> >>>> From what I've studied, I believe the Coding Standard and Google Java >>> Style >>>> Guide will be compatible, but we could decide that the Coding Standard >>>> supersedes anything in the Google Java Style Guide that is directly in >>>> conflict just in case. >>>> >>>> I'm not proposing we rigidly and blindly follow this Coding Standard. >> We >>>> can extend or even supersede portions of the adopted Coding Standard >> with >>>> our own Addendum. The Coding Standard Addendum would exist on the >> Apache >>>> Geode Wiki to define Geode-specific rules or recommendations. What I'd >>> like >>>> to see happen is for us to use the SEI CERT Coding Standard for Java >> as a >>>> starting point for our own Coding Standard. The resulting Coding >> Standard >>>> for Geode can be as static or as living and evolving as we wish. >>>> >>>> The Coding Standard can then provide helpful guidance in how we reshape >>>> some of the Geode code base that is in greater need of refactoring. It >>>> would also help guide us from following poor examples in the current >> code >>>> base when introducing new code. >>>> >>>> [1] https://cwiki.apache.org/confluence/display/GEODE/Code+Style+Guide >>>> [2] https://google.github.io/styleguide/javaguide.html >>>> [3] >>>> >>>> >>> >> https://wiki.sei.cmu.edu/confluence/display/seccode/SEI+CERT+Coding+Standards >>>> [4] >>>> >>>> >>> >> https://wiki.sei.cmu.edu/confluence/display/java/SEI+CERT+Oracle+Coding+Standard+for+Java >>>> >>> >>
