-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
