Val.
> But then you said that common sense doesn't work. Code review IS common sense
> for the most part though.
Code review must catch errors like "No validation in the world prevent me from
typing manually "mess" instead of «msg»"
But, when one reviewer prefers `qry` and another requires `qu
Folks,
I suppose we should rollout voting regarding abbreviations in Ignite 3.
BTW, are you actually aware of any other project defining something similar to
our abbreviations?
Sent from my iPhone
> On 23 Jun 2021, at 14:53, Nikita Ivanov wrote:
>
> What's worked for countless companies is t
What's worked for countless companies is the combination of the following:
1. Well defined rules (abbreviations, code styles & documentation, code
organization, etc., etc.) for common/frequent use cases.
2. Some basic tooling (wherever possible) plus a strong culture of code
reviews.
3. And... comm
Nikolay,
I'm confused. Konstantin gave you an example of a bad abbreviation that
cannot be caught by automated validation, and you referred to the code
review as a solution. But then you said that common sense doesn't work.
Code review IS common sense for the most part though.
Either way, I'm not
Hello, Pavel
> But it doesn't mean that now we need to use only full names.
> Abbreviations may be used e.g. for local variables with short scope.
> Here we must follow common sense and existing practices of effective naming.
> It shouldn't be a cargo cult as it is now.
But «common sense» has
>> But do you have a plan what should we do with the subject?
I think we need to just turn off the required rule for abbreviations and
start to write new code without this requirement.
But it doesn't mean that now we need to use only full names. Abbreviations
may be used e.g. for local variables
Hello!
42 Author: shroman
36 Author: Raul Kripalani
30 Author: samaitra
11 Author: Igor Rudyak
I believe that nobody in this list has worked on Ignite on behalf of
Sberbank or GridGain.
Regards,
--
Ilya Kasnacheev
вс, 20 июн. 2021 г. в 14:13, Pavel Kovalenko :
> I thi
Hi Pavel,
Thank you for sharing your thoughts! My personal opinion is very
similar. But do you have a plan what should we do with the subject? Do
you suggest to cleanup whole codebase from abbreviations? Or do you
think that it is fine to keep existing code as is and avoid
abbreviations for new co
I think in this discussion there should be a question - why do we still
need abbreviations for all variables and fields? What problem does it solve?
Nobody still clearly answered this question.
I saw only 2 arguments to keep abbreviations:
1) Less codebase symbols. I think it is a weak argument. We
> 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
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 написал(а):
>
> I'm
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 field
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 mak
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, Николай Ижиков :
> Hello, Ivan.
>
> We can create checkstyle rule to enforce usage of
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 написал(а):
>
> Nikita,
>
> Do you have a plan in your mind how to
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 exa
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-le
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
wrote:
>
> Konstantin, th
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 identifyi
+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 wrote:
>
> Hello, Anton, Alexei.
>
> Thanks for the feedback.
>
> Personally, I’m pre
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
> написал(а):
>
> -1
> Common abbrevs add quality to the code.
>
> пн, 7
-1
Common abbrevs add quality to the code.
пн, 7 июн. 2021 г. в 12:38, Anton Vinogradov :
> -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 (hardco
-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.
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 wrote:
> Hello, guys.
>
> Thanks for the feedback.
>
> Dmitry,
>
> > Manual rule enf
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 a
+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 wrote:
> In my opinion, we should remove this rule.
> Look
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 wrote:
> Hello, Igniters.
>
> Right now, we have the rule to use some predefined list of abbrevation for
> variable names
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] a
28 matches
Mail list logo
