I have a couple of questions/comments (in no particular order):
* When we reference the "Sun Java coding conventions" can we have a canonical link to that so that people don't have to make an assumption or try and find the version we're talking about? Are we referring to the (now Oracle) version here, or something else? https://www.oracle.com/java/technologies/javase/codeconventions-contents.html * I would recommend that we strengthen the recommendation for using enums for Boolean properties for any type that is used in method parameters. In my experience the improvement in readability at the call site outweighs the (modest, IMHO) cost of introducing a new enum, and the enum also provides a useful "handle" for providing documentation on the semantics of the flag. There are already a lot of Boolean parameters in use in the codebase and I can take a look at what it would take to clean these up * I like the section on Method clarity, but I would also call out non-trivial predicate logic as a candidate for encapsulation in its own method * Should we consider @NotNull/@Nullable or other annotations besides @Override? * In the exception handling section should we discuss using the most applicable exception type for the handler? I.e. don't catch Exception or Throwable? This probably falls under the don't silently swallow or log exceptions paragraph * The guidance on brace placement seems to contradict the Java coding conventions if we place the opening brace on a new line. Is that intentional or am I misreading the statement? Would it be clearer to link to a specific style as defined somewhere (e.g. https://en.wikipedia.org/wiki/Indentation_style#Variant:_Java) * The doc doesn't seem to cover a recommendation for braces with single-line bodies of conditional/loop statements. In my own experience it makes it easier to read if we uniformly used braces everywhere, but it does look like there are quite a few places in the code where we have unbraced ifs Overall the doc is well written and carefully considered, and I appreciate all of the work that went into it! Cheers, Derek From: "bened...@apache.org" <bened...@apache.org> Reply-To: "dev@cassandra.apache.org" <dev@cassandra.apache.org> Date: Friday, May 13, 2022 at 6:41 AM To: "dev@cassandra.apache.org" <dev@cassandra.apache.org> Subject: RE: [EXTERNAL]Updating our Code Contribution/Style Guide CAUTION: This email originated from outside of the organization. Do not click links or open attachments unless you can confirm the sender and know the content is safe. It’s been a couple of months since I opened this discussion. I think I have integrated the feedback into the google doc. Are there any elements anyone wants to continue discussing, or things I have not fully addressed? I’ll take an absence of response as lazy consensus to commit the changes to the wiki. From: bened...@apache.org <bened...@apache.org> Date: Monday, 14 March 2022 at 09:41 To: dev@cassandra.apache.org <dev@cassandra.apache.org> Subject: Updating our Code Contribution/Style Guide Our style guide hasn’t been updated in about a decade, and I think it is overdue some improvements that address some shortcomings as well as modern facilities such as streams and lambdas. Most of this was put together for an effort Dinesh started a few years ago, but has languished since, in part because the project has always seemed to have other priorities. I figure there’s never a good time to raise a contended topic, so here is my suggested update to contributor guidelines: https://docs.google.com/document/d/1sjw0crb0clQin2tMgZLt_ob4hYfLJYaU4lRX722htTo Many of these suggestions codify norms already widely employed, sometimes in spite of the style guide, but some likely remain contentious. Some potentially contentious things to draw your attention to: * Deemphasis of getX() nomenclature, in favour of richer set of prefixes and more succinct simple x() to retrieve where clear * Avoid implementing methods, incl. equals(), hashCode() and toString(), unless actually used * Modified new-line rules for multi-line function calls * External dependency rules (require DISCUSS thread before introducing)
