+1 to having a recommended reading list. At first glance, the SEI standard seems like an extremely useful resource, but I am hesitant to adopt it as our "coding standard" without carefully reading all the way through it. I would, however, be comfortable adding it to a recommended reading list.
- Aaron On Mon, Jun 24, 2019 at 4:28 PM Owen Nichols <onich...@pivotal.io> wrote: > 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 > >>>> > >>> > >> > >
