I'm sorry, but the rule is not strict and, moreover, not clear enough for constans. See here [1] ``` Type and method names are usually not abbreviated (except for the well-accepted abbreviations such as EOF, Impl, Fifo, etc.).
Abbreviation applies to local variables, method parameters, class fields and in **some cases public static fileds** (constants) of the class. ``` But there is not any list that contains these cases. I suppose, that constant naming should not be abbreviated. As I see, the most cases of violations, described in initial post, are about constant's namings. I suppose that we should define strict and clear rules about constants naming. [1] -- https://cwiki.apache.org/confluence/display/IGNITE/Abbreviation+Rules чт, 17 июн. 2021 г. в 10:10, Konstantin Orlov <kor...@gridgain.com>: > Enforced validation doesn't guarantee code consistency. No validation in > the world prevent me from typing manually "mess" instead of "msg"/"message" > or "svc" instead of "srvc"/"service" (btw "svc" is more common imo). > > And the fact that someone name a variable "count" instead of "cnt" doesn't > make the whole project immature. > > -- > Regards, > Konstantin Orlov > > > > > > On 17 Jun 2021, at 08:34, Ivan Pavlukhin <vololo...@gmail.com> wrote: > > > > Hi Nikolay, > > > > Thanks, it is rather interesting. > > > > Do we all agree that using different conventions for different code > > packages does not break "consistency"? Or did I get something wrong? > > > > 2021-06-17 7:12 GMT+03:00, Николай Ижиков <nizhi...@apache.org>: > >> Hello, Ivan. > >> > >> We can create checkstyle rule to enforce usage of abbreviations. > >> Internal/public code differs by package. > >> > >> PoC of rule [1] > >> > >> [1] https://github.com/apache/ignite/pull/9153 > >> > >>> 17 июня 2021 г., в 07:01, Ivan Pavlukhin <vololo...@gmail.com> > >>> написал(а): > >>> > >>> Nikita, > >>> > >>> Do you have a plan in your mind how to make Ignite codebase consistent? > >>> > >>> AFAIR, we had it intentionally inconsistent for a long time at least > >>> for one sake: for internal code we used special conventions (e.g. > >>> abbreviations, shorten getters) and common Java conventions for public > >>> API and examples (e.g. no abbreviations and usual getters). > >>> > >>> 2021-06-16 23:37 GMT+03:00, Nikita Ivanov <niva...@gridgain.com>: > >>>> Consistency is what makes it easier to contribute to the project and > >>>> attract new members. Consistency implies strong, well defined and > >>>> universally enforced rules. Just because we have some individuals who > >>>> are lazy or inexperienced - it does not mean that the entire project > >>>> should relax the basic-level engineering discipline. > >>>> > >>>> On a personal node - nothing screams "immaturity" louder that a code > >>>> that uses inconsistent naming, commenting, code style & organization, > >>>> etc. > >>>> -- > >>>> Nikita Ivanov > >>>> > >>>>> On Wed, Jun 16, 2021 at 5:56 AM Andrey Gura <ag...@apache.org> > wrote: > >>>>> > >>>>> Hi, > >>>>> > >>>>> I understand that this rule seems too strict or may be weird. But > >>>>> removing the rule could lead to review comments like "use future > >>>>> instead of fut". So my proposal is to change rule from "required" to > >>>>> "recommended". > >>>>> > >>>>> On Wed, Jun 16, 2021 at 2:49 PM Valentin Kulichenko > >>>>> <valentin.kuliche...@gmail.com> wrote: > >>>>>> > >>>>>> Konstantin, thanks for chiming in. > >>>>>> > >>>>>> That's exactly my concern. Overformalization is generally not a good > >>>>>> thing. > >>>>>> Someone used "mess" to abbreviate "message"? That is surely not a > good > >>>>>> coding style, but that's what code reviews are for. I believe that > our > >>>>>> committers are more than capable of identifying such issues. > >>>>>> > >>>>>> At the same time, with the current rules, we are *forced* to use > >>>>>> abbreviations like "locAddrGrpMgr", whether we like it or not. All > it > >>>>>> does > >>>>>> is makes it harder to contribute to Ignite, without providing any > >>>>>> clear > >>>>>> value. > >>>>>> > >>>>>> -Val > >>>>>> > >>>>>> On Thu, Jun 10, 2021 at 9:46 AM Konstantin Orlov < > kor...@gridgain.com> > >>>>>> wrote: > >>>>>> > >>>>>>> +1 for making this optional > >>>>>>> > >>>>>>> Common abbreviation rules is good for sure, but sometimes I getting > >>>>>>> sick > >>>>>>> of all those locAddrGrpMgr. > >>>>>>> > >>>>>>> > >>>>>>> -- > >>>>>>> Regards, > >>>>>>> Konstantin Orlov > >>>>>>> > >>>>>>> > >>>>>>> > >>>>>>> > >>>>>>>> On 7 Jun 2021, at 14:31, Nikolay Izhikov <nizhi...@apache.org> > >>>>>>>> wrote: > >>>>>>>> > >>>>>>>> Hello, Anton, Alexei. > >>>>>>>> > >>>>>>>> Thanks for the feedback. > >>>>>>>> > >>>>>>>> Personally, I’m pretty happy current abbreviation rules too. > >>>>>>>> Let see what we can do to make our codebase even more consistent. > >>>>>>>> > >>>>>>>>> 7 июня 2021 г., в 13:23, Alexei Scherbakov < > >>>>>>> alexey.scherbak...@gmail.com> написал(а): > >>>>>>>>> > >>>>>>>>> -1 > >>>>>>>>> Common abbrevs add quality to the code. > >>>>>>>>> > >>>>>>>>> пн, 7 июн. 2021 г. в 12:38, Anton Vinogradov <a...@apache.org>: > >>>>>>>>> > >>>>>>>>>> -1 here. > >>>>>>>>>> We can fix the code and set up the rule. > >>>>>>>>>> > >>>>>>>>>> This will help to prevent having a weird abbreviation like > "mess" > >>>>>>>>>> (from > >>>>>>>>>> "message") or "ign" (from "Ignite"). > >>>>>>>>>> Also, the abbreviations list (hardcoded at IDEA plugin) allows > to > >>>>>>>>>> have > >>>>>>> same > >>>>>>>>>> names across the whole code, this should simplify the reading. > >>>>>>>>>> > >>>>>>>>>> On Sat, Jun 5, 2021 at 10:49 PM Valentin Kulichenko < > >>>>>>>>>> valentin.kuliche...@gmail.com> wrote: > >>>>>>>>>> > >>>>>>>>>>> I also support removing this requirement. It’s not the first > >>>>>>>>>>> time > >>>>>>> someone > >>>>>>>>>>> brings this up, and so far we haven’t been able to fix it. Not > >>>>>>>>>>> worth > >>>>>>> it > >>>>>>>>>> in > >>>>>>>>>>> my view. > >>>>>>>>>>> > >>>>>>>>>>> -Val > >>>>>>>>>>> > >>>>>>>>>>> On Sat, Jun 5, 2021 at 11:54 AM Nikolay Izhikov > >>>>>>>>>>> <nizhi...@apache.org> > >>>>>>>>>>> wrote: > >>>>>>>>>>> > >>>>>>>>>>>> Hello, guys. > >>>>>>>>>>>> > >>>>>>>>>>>> Thanks for the feedback. > >>>>>>>>>>>> > >>>>>>>>>>>> Dmitry, > >>>>>>>>>>>> > >>>>>>>>>>>>> Manual rule enforcement saves a couple of symbols in code > >>>>>>>>>>>> > >>>>>>>>>>>> I’m talking about automatic check. > >>>>>>>>>>>> We can implement it easily. > >>>>>>>>>>>> So, if we decide to keep this rule all we need is to fix > >>>>>>>>>>>> current > >>>>>>>>>>>> violations (several thousand!). > >>>>>>>>>>>> After it, checkstyle will automatically enforce this rule. > >>>>>>>>>>>> As you may know, currently, any PR checked according to our > >>>>>>> checkstyle > >>>>>>>>>>>> rules. > >>>>>>>>>>>> Please, take a look at little green check sign after PR name. > >>>>>>>>>>>> > >>>>>>>>>>>> > >>>>>>>>>>>>> 5 июня 2021 г., в 00:57, Dmitry Pavlov <dpav...@apache.org> > >>>>>>>>>>> написал(а): > >>>>>>>>>>>>> > >>>>>>>>>>>>> +1 for removal. Cnt and count both seem to be fine. > >>>>>>>>>>>>> > >>>>>>>>>>>>> Manual rule enforcement saves a couple of symbols in code, > but > >>>>>>>>>> requires > >>>>>>>>>>>> some time from both contributor and reviewer. > >>>>>>>>>>>>> > >>>>>>>>>>>>> Sincerely, > >>>>>>>>>>>>> Dmitriy Pavlov > >>>>>>>>>>>>> > >>>>>>>>>>>>> On 2021/06/04 19:18:37, Pavel Tupitsyn <ptupit...@apache.org > > > >>>>>>> wrote: > >>>>>>>>>>>>>> In my opinion, we should remove this rule. > >>>>>>>>>>>>>> Looks like a waste of time. freq or frequency, cnt or count, > >>>>>>>>>>>>>> it is > >>>>>>>>>>> fine > >>>>>>>>>>>>>> either way. > >>>>>>>>>>>>>> > >>>>>>>>>>>>>> On Fri, Jun 4, 2021 at 7:39 PM Nikolay Izhikov < > >>>>>>> nizhi...@apache.org > >>>>>>>>>>> > >>>>>>>>>>>> wrote: > >>>>>>>>>>>>>> > >>>>>>>>>>>>>>> Hello, Igniters. > >>>>>>>>>>>>>>> > >>>>>>>>>>>>>>> Right now, we have the rule to use some predefined list of > >>>>>>>>>>> abbrevation > >>>>>>>>>>>> for > >>>>>>>>>>>>>>> variable names [1]. > >>>>>>>>>>>>>>> Some of the reviewers ask to follow this rule strictly. > >>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>> It is required to use abbreviated form for code > >>>>>>>>>>>>>>>> consistency. > >>>>>>>>>>>>>>> > >>>>>>>>>>>>>>> I tried to implement this rule in form of checkstyle check > >>>>>>>>>>>>>>> [2] and > >>>>>>>>>> it > >>>>>>>>>>>>>>> seems like many of use doesn’t follow this requirement. > >>>>>>>>>>>>>>> My check found 4124 violation in core module. > >>>>>>>>>>>>>>> > >>>>>>>>>>>>>>> Should we make this rule optional in documentation or > should > >>>>>>>>>>>>>>> we > >>>>>>>>>>> remove > >>>>>>>>>>>> it > >>>>>>>>>>>>>>> completely? > >>>>>>>>>>>>>>> Or should someone proceed and fix all the violations? > >>>>>>>>>>>>>>> > >>>>>>>>>>>>>>> WDYT? > >>>>>>>>>>>>>>> > >>>>>>>>>>>>>>> > >>>>>>>>>>>>>>> Example of output: > >>>>>>>>>>>>>>> ``` > >>>>>>>>>>>>>>> [ERROR] > >>>>>>>>>>>>>>> > >>>>>>>>>>>> > >>>>>>>>>>> > >>>>>>>>>> > >>>>>>> > /Users/sbt-izhikov-nv/work/ignite/modules/core/src/test/java/org/apache/ignite/internal/processors/cache/persistence/db/IgnitePdsWithTtlTest.java:94: > >>>>>>>>>>>>>>> Abbrevation should be used for CACHE_NAME_LOCAL_ATOMIC! > >>>>>>>>>>>>>>> Please, > >>>>>>> use > >>>>>>>>>>>> loc, > >>>>>>>>>>>>>>> instead of LOCAL [IgniteAbbrevationsRule] > >>>>>>>>>>>>>>> [ERROR] > >>>>>>>>>>>>>>> > >>>>>>>>>>>> > >>>>>>>>>>> > >>>>>>>>>> > >>>>>>> > /Users/sbt-izhikov-nv/work/ignite/modules/core/src/test/java/org/apache/ignite/internal/processors/cache/persistence/db/IgnitePdsWithTtlTest.java:97: > >>>>>>>>>>>>>>> Abbrevation should be used for CACHE_NAME_LOCAL_TX! Please, > >>>>>>>>>>>>>>> use > >>>>>>>>>> loc, > >>>>>>>>>>>>>>> instead of LOCAL [IgniteAbbrevationsRule] > >>>>>>>>>>>>>>> [ERROR] > >>>>>>>>>>>>>>> > >>>>>>>>>>>> > >>>>>>>>>>> > >>>>>>>>>> > >>>>>>> > /Users/sbt-izhikov-nv/work/ignite/modules/core/src/test/java/org/apache/ignite/internal/processors/cache/persistence/db/IgnitePdsWithTtlTest.java:264: > >>>>>>>>>>>>>>> Abbrevation should be used for checkpointManager! Please, > use > >>>>>>>>>>>>>>> mgr, > >>>>>>>>>>>> instead > >>>>>>>>>>>>>>> of Manager [IgniteAbbrevationsRule] > >>>>>>>>>>>>>>> [ERROR] > >>>>>>>>>>>>>>> > >>>>>>>>>>>> > >>>>>>>>>>> > >>>>>>>>>> > >>>>>>> > /Users/sbt-izhikov-nv/work/ignite/modules/core/src/test/java/org/apache/ignite/internal/processors/cache/persistence/db/IgnitePdsPartitionPreloadTest.java:63: > >>>>>>>>>>>>>>> Abbrevation should be used for DEFAULT_REGION! Please, use > >>>>>>>>>>>>>>> dflt, > >>>>>>>>>>>> instead of > >>>>>>>>>>>>>>> DEFAULT [IgniteAbbrevationsRule] > >>>>>>>>>>>>>>> [ERROR] > >>>>>>>>>>>>>>> > >>>>>>>>>>>> > >>>>>>>>>>> > >>>>>>>>>> > >>>>>>> > /Users/sbt-izhikov-nv/work/ignite/modules/core/src/test/java/org/apache/ignite/internal/processors/cache/persistence/db/IgnitePdsWholeClusterRestartTest.java:47: > >>>>>>>>>>>>>>> Abbrevation should be used for ENTRIES_COUNT! Please, use > >>>>>>>>>>>>>>> cnt, > >>>>>>>>>>> instead > >>>>>>>>>>>> of > >>>>>>>>>>>>>>> COUNT [IgniteAbbrevationsRule] > >>>>>>>>>>>>>>> [ERROR] > >>>>>>>>>>>>>>> > >>>>>>>>>>>> > >>>>>>>>>>> > >>>>>>>>>> > >>>>>>> > /Users/sbt-izhikov-nv/work/ignite/modules/core/src/test/java/org/apache/ignite/internal/processors/cache/persistence/db/IgnitePdsRebalancingOnNotStableTopologyTest.java:49: > >>>>>>>>>>>>>>> Abbrevation should be used for CHECKPOINT_FREQUENCY! > Please, > >>>>>>>>>>>>>>> use > >>>>>>>>>>> freq, > >>>>>>>>>>>>>>> instead of FREQUENCY [IgniteAbbrevationsRule] > >>>>>>>>>>>>>>> [ERROR] > >>>>>>>>>>>>>>> > >>>>>>>>>>>> > >>>>>>>>>>> > >>>>>>>>>> > >>>>>>> > /Users/sbt-izhikov-nv/work/ignite/modules/core/src/test/java/org/apache/ignite/internal/processors/cache/persistence/db/IgnitePdsTransactionsHangTest.java:75: > >>>>>>>>>>>>>>> Abbrevation should be used for MAX_KEY_COUNT! Please, use > >>>>>>>>>>>>>>> cnt, > >>>>>>>>>>> instead > >>>>>>>>>>>> of > >>>>>>>>>>>>>>> COUNT [IgniteAbbrevationsRule] > >>>>>>>>>>>>>>> [ERROR] > >>>>>>>>>>>>>>> > >>>>>>>>>>>> > >>>>>>>>>>> > >>>>>>>>>> > >>>>>>> > /Users/sbt-izhikov-nv/work/ignite/modules/core/src/test/java/org/apache/ignite/internal/processors/cache/persistence/db/IgnitePdsTransactionsHangTest.java:78: > >>>>>>>>>>>>>>> Abbrevation should be used for CHECKPOINT_FREQUENCY! > Please, > >>>>>>>>>>>>>>> use > >>>>>>>>>>> freq, > >>>>>>>>>>>>>>> instead of FREQUENCY [IgniteAbbrevationsRule] > >>>>>>>>>>>>>>> [ERROR] > >>>>>>>>>>>>>>> > >>>>>>>>>>>> > >>>>>>>>>>> > >>>>>>>>>> > >>>>>>> > /Users/sbt-izhikov-nv/work/ignite/modules/core/src/test/java/org/apache/ignite/internal/processors/cache/persistence/db/SlowHistoricalRebalanceSmallHistoryTest.java:57: > >>>>>>>>>>>>>>> Abbrevation should be used for SUPPLY_MESSAGE_LATCH! > Please, > >>>>>>>>>>>>>>> use > >>>>>>>>>> msg, > >>>>>>>>>>>>>>> instead of MESSAGE [IgniteAbbrevationsRule] > >>>>>>>>>>>>>>> [ERROR] > >>>>>>>>>>>>>>> > >>>>>>>>>>>> > >>>>>>>>>>> > >>>>>>>>>> > >>>>>>> > /Users/sbt-izhikov-nv/work/ignite/modules/core/src/test/java/org/apache/ignite/internal/processors/cache/persistence/db/IgniteLogicalRecoveryTest.java:74: > >>>>>>>>>>>>>>> Abbrevation should be used for SHARED_GROUP_NAME! Please, > use > >>>>>>>>>>>>>>> grp, > >>>>>>>>>>>> instead > >>>>>>>>>>>>>>> of GROUP [IgniteAbbrevationsRule] > >>>>>>>>>>>>>>> [ERROR] > >>>>>>>>>>>>>>> > >>>>>>>>>>>> > >>>>>>>>>>> > >>>>>>>>>> > >>>>>>> > /Users/sbt-izhikov-nv/work/ignite/modules/core/src/test/java/org/apache/ignite/internal/processors/cache/persistence/db/IgniteLogicalRecoveryTest.java:200: > >>>>>>>>>>>>>>> Abbrevation should be used for cacheLoader! Please, use > ldr, > >>>>>>>>>> instead > >>>>>>>>>>> of > >>>>>>>>>>>>>>> Loader [IgniteAbbrevationsRule] > >>>>>>>>>>>>>>> ``` > >>>>>>>>>>>>>>> > >>>>>>>>>>>>>>> [1] > >>>>>>>>>>>>>>> > >>>>>>>>>>>> > >>>>>>>>>>> > >>>>>>>>>> > >>>>>>> > https://cwiki.apache.org/confluence/display/IGNITE/Abbreviation+Rules#AbbreviationRules-VariableAbbreviation > >>>>>>>>>>>>>>> [2] https://github.com/apache/ignite/pull/9153 > >>>>>>>>>>>>>>> > >>>>>>>>>>>>>>> > >>>>>>>>>>>>>> > >>>>>>>>>>>> > >>>>>>>>>>>> > >>>>>>>>>>> > >>>>>>>>>> > >>>>>>>>> > >>>>>>>>> > >>>>>>>>> -- > >>>>>>>>> > >>>>>>>>> Best regards, > >>>>>>>>> Alexei Scherbakov > >>>>>>>> > >>>>>>> > >>>>>>> > >>>> > >>> > >>> > >>> -- > >>> > >>> Best regards, > >>> Ivan Pavlukhin > >> > > > > > > -- > > > > Best regards, > > Ivan Pavlukhin > > -- Sincerely yours, Ivan Daschinskiy
