+1 to the guideline.
> > For the instance() / getInstance() methods - I know it is an additional > effort, but on the other hand it has many advantages because you can > replace the singleton for testing > > Again, do this as necessary. I think for public instances this is a fine > recommendation, but for private uses it should not be prescribed, only used > if there is an explicit benefit. It is regarding testability. Where mock is desired, there should be getter methods, instead of 'public final'. Otherwise, `public final` is preferred for its simplicity. It is more tricky in terms of singletons though. I feel there is no good use of private singleton, which is ugly and makes the referencing code difficult to test. So probably for singleton, we want to declare the 'instance()' method. It is good that the guideline is not rigid. I don’t think it is good idea to prohibit or discourage to use final, which > is a tool to guard immutability. Ruslan, What is proposed is to prohibit or discourage the use of `final` *within a method body*. I think it is less useful to mark a variable's *reference* as being immutable within such scope. In the other scenario, e.g. class member fields, `final` should be used when reference/primitive immutability is desired. - Yifan On Tue, Mar 15, 2022 at 3:46 AM Ruslan Fomkin <ruslan.fom...@gmail.com> wrote: > Hi, > > I hope it’s OK I jump to the discussion. > > I find it is important to automate code formatting and have a build check > to verify it, otherwise there are many examples in other projects that > formatting is not followed. To make formatting to be not painful for > contributors it will be good to setup git commit hooks (which will require > to have a command line formatting tool) in addition to IDE support. In such > case the main task for the formatting CI build check will be to fail > environments, which are not yet set. > For example, cassandra-dtest already has a CI formatting check in place > for Python code, which runs on each PR. There is a Python formatting > command line tool, which can be easily run locally, and if I don’t mistake > it is easy to setup git commit hook with it. (also works to setup the > formatting in VScode) > > I don’t think it is good idea to prohibit or discourage to use final, > which is a tool to guard immutability. As mentioned unfortunately Java is > not designed to be safe by default and thus makes code more noisy by > requiring to use the keyword. > > I noticed an issue with current formatting that there is no indentation if > an assignment statement is split to multiple lines before or without using > parenthesis. For example: > ImmutableMap.Builder<String, ImmutableMultimap<String, > InetAddressAndPort>> dcRackBuilder = > ImmutableMap.builder(); > It would be nice if the next line is intended to understand that it is > part of the previous line. > > I support Jacek’s request to have each argument on a separate line when > they are many and need to be placed on multiple lines. For me it takes less > effort to grasp arguments on separate lines than when several arguments are > combined on the same line. IMHO the root cause is having too many > arguments, which is common issue for non-OOP languages. > > Best regards, > Ruslan Fomkin > > On 15 Mar 2022, at 10:04, Stefan Miklosovic < > stefan.mikloso...@instaclustr.com> wrote: > > I agree with the single commit approach to fix it all. TBH Javadocs > are a little bit messy as well, warnings on generating them, > incomplete, in a lot of cases obsolete or they do not reflect the code > anymore etc. > > On Tue, 15 Mar 2022 at 09:44, bened...@apache.org <bened...@apache.org> > wrote: > > > I’d be fine with that, though I think if we want to start enforcing > imports we probably want to mass correct them first. It’s not like other > style requirements in that there should not be unintended consequences. A > single (huge) commit to standardise the orders and introduce a build-time > check would be fine IMO. > > > > I also don’t really think it is that important. > > > > From: Jacek Lewandowski <lewandowski.ja...@gmail.com> > Date: Tuesday, 15 March 2022 at 05:18 > To: dev@cassandra.apache.org <dev@cassandra.apache.org> > Subject: Re: Updating our Code Contribution/Style Guide > > I do think that we should at least enforce the import order. What is now > is a complete mess and causes a lot of conflicts during rebasing / merging. > Perhaps we could start enforcing such rules only on modified files, this > way we could gradually go towards consistency... wdyt? > > > - - -- --- ----- -------- ------------- > Jacek Lewandowski > > > > > > On Tue, Mar 15, 2022 at 1:52 AM Dinesh Joshi <djo...@apache.org> wrote: > > Benedict, I agree. We should not be rigid about applying any style. > stylechecks are meant to bring uniformity in the codebase. I assure you > what I am proposing is neither rigid nor curbs the ability to apply the > rules flexibly. > > > > On Mar 14, 2022, at 4:52 PM, bened...@apache.org wrote: > > > > I’m a strong -1 on strictly enforcing any style guide. It is there to help > shape contributions, review feedback and responding to said feedback. It > can also be used to setup IntelliJ’s code formatter to configure default > behaviours. > > > > It is not meant to be turned into a linter. Plenty of the rules are stated > in a flexible manner, so as to permit breaches where overall legibility and > aesthetics are improved. > > > > > > From: Dinesh Joshi <djo...@apache.org> > Date: Monday, 14 March 2022 at 23:44 > To: dev@cassandra.apache.org <dev@cassandra.apache.org> > Subject: Re: Updating our Code Contribution/Style Guide > > I am also in favor of updating the style guide. We should ideally have > custom checkstyle configuration that can ensure adherence to the style > guide. > > > > I also don't think this is a contended topic. We want to explicitly codify > our current practices so new contributors have an easier time writing code. > > > > It is also important to note that the current codebase is not consistent > since it was written over a long period of time so it tends to confuse > folks who are working in different parts of the codebase. So this style > guide would be very helpful. > > > > On Mar 14, 2022, at 2:41 AM, bened...@apache.org wrote: > > > > 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) > > > >
