> Agree. I think you can start a discussion and change some abbrevs if you wish.
How about keeping the abbreviation map file inside the Ignite repo? In this case, you may change the abbreviation by issue/pr covered by a green TC check. On Thu, Jun 17, 2021 at 11:36 AM Nikolay Izhikov <nizhi...@apache.org> wrote: > > No validation in the world prevent me from typing manually "mess" > instead of "msg"/«message" > > Code review will do. > > > btw "svc" is more common imo > > Agree. I think you can start a discussion and change some abbrevs if you > wish. > > > > 17 июня 2021 г., в 10:23, Ivan Daschinsky <ivanda...@gmail.com> > написал(а): > > > > I'm sorry, but the rule is not strict and, moreover, not clear enough for > > constans. See here [1] > > ``` > > Type and method names are usually not abbreviated (except for the > > well-accepted abbreviations such as EOF, Impl, Fifo, etc.). > > > > Abbreviation applies to local variables, method parameters, class fields > > and in **some cases public static fileds** (constants) of the class. > > ``` > > But there is not any list that contains these cases. I suppose, that > > constant naming should not be abbreviated. > > As I see, the most cases of violations, described in initial post, are > > about constant's namings. > > > > I suppose that we should define strict and clear rules about constants > > naming. > > > > [1] -- > https://cwiki.apache.org/confluence/display/IGNITE/Abbreviation+Rules > > > > > > чт, 17 июн. 2021 г. в 10:10, Konstantin Orlov <kor...@gridgain.com>: > > > >> Enforced validation doesn't guarantee code consistency. No validation in > >> the world prevent me from typing manually "mess" instead of > "msg"/"message" > >> or "svc" instead of "srvc"/"service" (btw "svc" is more common imo). > >> > >> And the fact that someone name a variable "count" instead of "cnt" > doesn't > >> make the whole project immature. > >> > >> -- > >> Regards, > >> Konstantin Orlov > >> > >> > >> > >> > >>> On 17 Jun 2021, at 08:34, Ivan Pavlukhin <vololo...@gmail.com> wrote: > >>> > >>> Hi Nikolay, > >>> > >>> Thanks, it is rather interesting. > >>> > >>> Do we all agree that using different conventions for different code > >>> packages does not break "consistency"? Or did I get something wrong? > >>> > >>> 2021-06-17 7:12 GMT+03:00, Николай Ижиков <nizhi...@apache.org>: > >>>> Hello, Ivan. > >>>> > >>>> We can create checkstyle rule to enforce usage of abbreviations. > >>>> Internal/public code differs by package. > >>>> > >>>> PoC of rule [1] > >>>> > >>>> [1] https://github.com/apache/ignite/pull/9153 > >>>> > >>>>> 17 июня 2021 г., в 07:01, Ivan Pavlukhin <vololo...@gmail.com> > >>>>> написал(а): > >>>>> > >>>>> Nikita, > >>>>> > >>>>> Do you have a plan in your mind how to make Ignite codebase > consistent? > >>>>> > >>>>> AFAIR, we had it intentionally inconsistent for a long time at least > >>>>> for one sake: for internal code we used special conventions (e.g. > >>>>> abbreviations, shorten getters) and common Java conventions for > public > >>>>> API and examples (e.g. no abbreviations and usual getters). > >>>>> > >>>>> 2021-06-16 23:37 GMT+03:00, Nikita Ivanov <niva...@gridgain.com>: > >>>>>> Consistency is what makes it easier to contribute to the project and > >>>>>> attract new members. Consistency implies strong, well defined and > >>>>>> universally enforced rules. Just because we have some individuals > who > >>>>>> are lazy or inexperienced - it does not mean that the entire project > >>>>>> should relax the basic-level engineering discipline. > >>>>>> > >>>>>> On a personal node - nothing screams "immaturity" louder that a code > >>>>>> that uses inconsistent naming, commenting, code style & > organization, > >>>>>> etc. > >>>>>> -- > >>>>>> Nikita Ivanov > >>>>>> > >>>>>>> On Wed, Jun 16, 2021 at 5:56 AM Andrey Gura <ag...@apache.org> > >> wrote: > >>>>>>> > >>>>>>> Hi, > >>>>>>> > >>>>>>> I understand that this rule seems too strict or may be weird. But > >>>>>>> removing the rule could lead to review comments like "use future > >>>>>>> instead of fut". So my proposal is to change rule from "required" > to > >>>>>>> "recommended". > >>>>>>> > >>>>>>> On Wed, Jun 16, 2021 at 2:49 PM Valentin Kulichenko > >>>>>>> <valentin.kuliche...@gmail.com> wrote: > >>>>>>>> > >>>>>>>> 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 > >>>>>>>>>> > >>>>>>>>> > >>>>>>>>> > >>>>>> > >>>>> > >>>>> > >>>>> -- > >>>>> > >>>>> Best regards, > >>>>> Ivan Pavlukhin > >>>> > >>> > >>> > >>> -- > >>> > >>> Best regards, > >>> Ivan Pavlukhin > >> > >> > > > > -- > > Sincerely yours, Ivan Daschinskiy > >
