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