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 > > > >
