Maxim, > 1. The Apache Ignite may start multiple instances... Ignite codebase contains something more than Ignite instances code: control utility, tests. And it does not sound good to me to use some project-specific naming rule if it is not really critical. I wish Ignite coding guidelines are as common as possible in order simplify things for new contributors. I briefly examined code and found some static final ThreadLocals and Atomic variables. Both are not constants.
Currently my opinion is that the proposed change can make more harm than profit. Best regards, Ivan Pavlukhin пн, 27 апр. 2020 г. в 14:57, Maxim Muzafarov <mmu...@apache.org>: > > Pavel, Ivan, > > > Let me clarify some details you missed which may lead you to the light side. > > 1. The Apache Ignite may start multiple instances on the same VM, so > keeping a class field `static` for mutable objects have no sense in > most of the cases. In case of keeping, the immutable structure will be > shared between Ignite instances which happens in very rare and special > cases. According to this assumption, most of `final static` fields are > really constants. > Even thread-local objects probably better to keep `non-static` as > Alexey mentioned before [1]. > > 2. The common `final static` fields like - `log`, `logger`, > `serialVersionUID` added to the exclusion list. If it requires we can > extend it more. > > Yes, I know the java code naming convention mentioned above by myself, > but exactly for Ignite IMHO it's better to apply for all `static > final` fields naming as `constant naming`. > > [1] > http://apache-ignite-developers.2346864.n4.nabble.com/Class-field-ThreadLocal-Why-not-static-tp34878p34881.html > > On Mon, 27 Apr 2020 at 14:13, Anton Vinogradov <a...@apache.org> wrote: > > > > >> +1 to force upper case for `static final` variables. > > +1 too > > > > On Mon, Apr 27, 2020 at 12:08 PM Nikolay Izhikov <nizhi...@apache.org> > > wrote: > > > > > +1 to force upper case for `static final` variables. > > > > > > > 25 апр. 2020 г., в 07:39, Ivan Pavlukhin <vololo...@gmail.com> > > > написал(а): > > > > > > > > Maxim, > > > > > > > > Thank you for efforts in a code quality improvement! > > > > > > > > Unfortunately I do not agree with the proposal. Usually I like to > > > > refer a following guide [1]. While a question "what is a constant" is > > > > not trivial but I suppose that every static final field capitalization > > > > can cause some problems. For example, a developer can make false > > > > assumptions about immutability and thread safety basing on a > > > > capitalized field name. > > > > > > > > [1] > > > https://google.github.io/styleguide/javaguide.html#s5.2.4-constant-names > > > > > > > > Best regards, > > > > Ivan Pavlukhin > > > > > > > > пт, 24 апр. 2020 г. в 21:58, Pavel Tupitsyn <ptupit...@apache.org>: > > > >> > > > >>> All static final object reference types that are never followed by "." > > > >> (dot) > > > >> With this way of thinking we can say that everything is a constant if > > > >> we > > > >> don't change it - no need for static final. > > > >> "Constant" is usually something that you can't change, even you want - > > > >> compiler won't let you. > > > >> In Java that would be static final primitives and read-only objects > > > >> like > > > >> String. > > > >> > > > >> On Fri, Apr 24, 2020 at 8:25 PM Pavel Pereslegin <xxt...@gmail.com> > > > wrote: > > > >> > > > >>> Maxim, > > > >>> > > > >>>> But what exactly is `constant` means?! I'd suggest applying this rule > > > >>>> to all class fields with `static final` modifiers without any > > > >>>> clauses. > > > >>>> (check Java Naming convention [2] paragraph 3.3). > > > >>> > > > >>> I disagree with this and want to clarify what exactly the Java naming > > > >>> convention says: > > > >>> > > > >>>>> The following are considered to be constants: > > > >>>>> 1. All static final primitive types (Remember that all interface > > > fields > > > >>> are inherently static final). > > > >>>>> 2. All static final object reference types that are never followed > > > >>>>> by > > > >>> "." (dot). > > > >>>>> 3. All static final arrays that are never followed by"[" (dot) > > > >>> > > > >>> I don't see that convention says "any static final field". > > > >>> > > > >>> пт, 24 апр. 2020 г. в 20:11, Sergey Antonov <antonovserge...@gmail.com > > > >: > > > >>>> > > > >>>> Maxim, It's a good idea! > > > >>>> > > > >>>> Please don't forget to update out code style guidelines too. > > > >>>> > > > >>>> Thank you for keeping the code cleaner! > > > >>>> > > > >>>> пт, 24 апр. 2020 г. в 19:49, Maxim Muzafarov <mmu...@apache.org>: > > > >>>> > > > >>>>> Igniters, > > > >>>>> > > > >>>>> > > > >>>>> It is not directly mentioned through the Apache Ignite Coding > > > >>>>> Guidelines [1] about naming the `static final` class fields using > > > only > > > >>>>> upper-case letters. I'd like to suggest to fill this gap. > > > >>>>> > > > >>>>>> Constants should all be upper-case. > > > >>>>> But what exactly is `constant` means?! I'd suggest applying this > > > >>>>> rule > > > >>>>> to all class fields with `static final` modifiers without any > > > clauses. > > > >>>>> (check Java Naming convention [2] paragraph 3.3). > > > >>>>> > > > >>>>> > > > >>>>> I've prepared PR [3] with capitalizing letters on all of the > > > >>>>> constant > > > >>>>> names simultaneously with supporting the standard ConstantName > > > >>>>> checkstyle [4] rule. > > > >>>>> > > > >>>>> Can we proceed with this change? > > > >>>>> WDYT? > > > >>>>> > > > >>>>> > > > >>>>> [1] > > > >>>>> > > > >>> > > > https://cwiki.apache.org/confluence/display/IGNITE/Coding+Guidelines#CodingGuidelines-Naming > > > >>>>> [2] > > > >>>>> > > > >>> > > > https://web.archive.org/web/20120911192801/developers.sun.com/sunstudio/products/archive/whitepapers/java-style.pdf > > > >>>>> [3] https://github.com/apache/ignite/pull/7662 > > > >>>>> [4] https://issues.apache.org/jira/browse/IGNITE-12888 > > > >>>>> > > > >>>> > > > >>>> > > > >>>> -- > > > >>>> BR, Sergey Antonov > > > >>> > > > > > >
