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
