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

Reply via email to