Hi Vyacheslav, Can we proceed with plugin publishing? I have not seen any objections, so I propose to complete this task.
Sincerely, Dmitriy Pavlov вт, 8 мая 2018 г. в 21:47, Vyacheslav Daradur <daradu...@gmail.com>: > Hi, Igniters! > > After the completion of publishing abbr-plugin [1][2] we will be able > to automate checking of method arguments code style. > > It will be easy to check rules approved by the community during writing > code. > > [1] https://issues.apache.org/jira/browse/IGNITE-5698 > [2] > http://apache-ignite-developers.2346864.n4.nabble.com/abbrevation-rules-plugin-td19356.html > > On Tue, May 8, 2018 at 8:34 PM, Dmitry Pavlov <dpavlov....@gmail.com> > wrote: > > Folks, I've messed with another topic, where Vladimir was going to update > > review check-list. > > > > Here I've updated Coding Guidelines: > > > https://cwiki.apache.org/confluence/display/IGNITE/Coding+Guidelines#CodingGuidelines-MethodArguments > > > > Please review changes, so we can consider it is final. > > > > Sincerely, > > Dmitriy Pavlov > > > > вт, 8 мая 2018 г. в 20:05, Dmitry Pavlov <dpavlov....@gmail.com>: > > > >> I thought that Vladimir will update. > >> > >> By the way, Denis M, I propose to grant access to the wiki to Dmitry G. > >> WDYT? > >> > >> вт, 8 мая 2018 г. в 19:28, Dmitriy Govorukhin < > >> dmitriy.govoruk...@gmail.com>: > >> > >>> Dmitriy, > >>> > >>> Сould you please update code style wiki page in accordance with the > >>> results of the discussion? > >>> On May 7 2018, at 11:00 am, Vladimir Ozerov <voze...@gridgain.com> > wrote: > >>> > > >>> > Dmitry, > >>> > Agree, mixed style when some arguments share the same line and others > >>> don't > >>> > looks very bad. My proposal was to allow two styles - first when all > >>> > arguments are on the same line splitted by 120 char limit, second > when > >>> all > >>> > every arguments is on a separate line. > >>> > Mixed style should be disallowed. > >>> > > >>> > On Mon, May 7, 2018 at 12:35 AM, Dmitriy Govorukhin < > >>> > dmitriy.govoruk...@gmail.com> wrote: > >>> > > >>> > > Vladimir, > >>> > > My eyes cry when I see this > >>> > > public double getCost(Session ses, int[] masks, TableFilter[] > filters, > >>> > > int filter, SortOrder sortOrder, > >>> > > HashSet<Column> cols) { > >>> > > return SpatialTreeIndex.getCostRangeIndex(masks,table. > >>> > > getRowCountApproximation(), > >>> > > columns) / 10; > >>> > > } > >>> > > > >>> > > Why did arguments split into 3 line? > >>> > > It is much better readable for me > >>> > > public double getCost( > >>> > > Session ses, > >>> > > int[] masks, > >>> > > TableFilter[] filters, > >>> > > int filter, > >>> > > SortOrder sortOrder, > >>> > > HashSet<Column> cols > >>> > > ) { > >>> > > return SpatialTreeIndex.getCostRangeIndex(masks, > >>> > > able.getRowCountApproximation(), > >>> > > columns) / 10; > >>> > > } > >>> > > > >>> > > Do we have a serious reason to continue writing code as before? > >>> > > > >>> > > > >>> > > On Thu, May 3, 2018 at 2:24 PM, Vladimir Ozerov < > voze...@gridgain.com > >>> > > >>> > > wrote: > >>> > > > >>> > > > My opinion is that we should allow both styles and not enforce > any > >>> of > >>> > > them. > >>> > > > I hardly can say that this > >>> > > > > >>> > > > public double getCost( > >>> > > > Session ses, > >>> > > > int[] masks, > >>> > > > TableFilter[] filters, > >>> > > > int filter, > >>> > > > SortOrder sortOrder, > >>> > > > HashSet<Column> cols > >>> > > > ) { > >>> > > > return SpatialTreeIndex.getCostRangeIndex(masks, > >>> > > > table.getRowCountApproximation(), columns) / 10; > >>> > > > } > >>> > > > > >>> > > > is better than this > >>> > > > public double getCost(Session ses, int[] masks, TableFilter[] > >>> filters, > >>> > > > int filter, SortOrder sortOrder, > >>> > > > HashSet<Column> cols) { > >>> > > > return SpatialTreeIndex.getCostRangeIndex(masks, > >>> > > > table.getRowCountApproximation(), columns) / 10; > >>> > > > } > >>> > > > > >>> > > > > >>> > > > But this > >>> > > > public CacheLockCandidates doneRemote( > >>> > > > GridCacheVersion ver, > >>> > > > Collection<GridCacheVersion> pending, > >>> > > > Collection<GridCacheVersion> committed, > >>> > > > Collection<GridCacheVersion> rolledback > >>> > > > ) > >>> > > > > >>> > > > > >>> > > > looks better than this > >>> > > > public CacheLockCandidates doneRemote(GridCacheVersion ver, > >>> > > > Collection<GridCacheVersion> pending, > >>> > > > Collection<GridCacheVersion> committed, > >>> > > > Collection<GridCacheVersion> rolledback) > >>> > > > > >>> > > > > >>> > > > The very problem is that our eyes feel comfortable when we either > >>> move > >>> > > them > >>> > > > horizontally, or vertically (example 2). But with proposed code > >>> style you > >>> > > > have to do zigzag movements in general case because lines are not > >>> aligned > >>> > > > (example 1). > >>> > > > Merge conflicts on multiliners are hardly of major concern for > us. > >>> > > > > >>> > > > Vladimir. > >>> > > > On Thu, May 3, 2018 at 1:46 PM, Eduard Shangareev < > >>> > > > eduard.shangar...@gmail.com> wrote: > >>> > > > > >>> > > > > Alexey, > >>> > > > > +1. > >>> > > > > I personally also follow this style. > >>> > > > > On Thu, May 3, 2018 at 12:45 PM, Alexey Goncharuk < > >>> > > > > alexey.goncha...@gmail.com> wrote: > >>> > > > > > >>> > > > > > Actually, I've been following the suggested code style for > >>> quite a > >>> > > > while. > >>> > > > > > I'm ok to add this to coding guidelines, however, I think we > >>> should > >>> > > > > > >>> > > > > >>> > > > allow > >>> > > > > > the old style when the method signature (without throws > clause) > >>> fits > >>> > > > > > >>> > > > > >>> > > > the > >>> > > > > > line. > >>> > > > > > > >>> > > > > > Thoughts? > >>> > > > > > 2018-05-03 12:09 GMT+03:00 Dmitry Pavlov < > dpavlov....@gmail.com > >>> >: > >>> > > > > > > Hi Dmitriy, > >>> > > > > > > I like your proposal, so +1 from me. > >>> > > > > > > I think it would make code more readable and easy to > >>> understand. > >>> > > > > > > Sincerely, > >>> > > > > > > Dmitriy Pavlov > >>> > > > > > > > >>> > > > > > > чт, 3 мая 2018 г. в 11:31, Dmitriy Govorukhin < > >>> > > > > > > dmitriy.govoruk...@gmail.com > >>> > > > > > > > : > >>> > > > > > > > >>> > > > > > > > >>> > > > > > > > Hi folks, > >>> > > > > > > > I read > >>> > > > > > > > https://cwiki.apache.org/confluence/display/IGNITE/ > >>> > > > > > > > >>> > > > > > > >>> > > > > > >>> > > > > >>> > > > Coding+Guidelines > >>> > > > > , > >>> > > > > > > > but did not find anything about code style for method > >>> arguments. > >>> > > > > > > > > >>> > > > > > > > In many places in the code, I see different code style, > this > >>> > > > creates > >>> > > > > > > > difficulties for reading. > >>> > > > > > > > > >>> > > > > > > > It seems to me an example below is rather difficult to > >>> perceive. > >>> > > > > > > > ```java > >>> > > > > > > > public void foo(Object obj1, > >>> > > > > > > > Object obj2, Object obj3,... ,Object objN){ > >>> > > > > > > > .... > >>> > > > > > > > } > >>> > > > > > > > ``` > >>> > > > > > > > An example GridCacheProcessor.addCacheOnJoin(...) > >>> > > > > > > > > >>> > > > > > > > ```java > >>> > > > > > > > private void addCacheOnJoin(CacheConfiguration<?, ?> cfg, > >>> > > > > > > > >>> > > > > > > >>> > > > > > >>> > > > > boolean > >>> > > > > > > sql, > >>> > > > > > > > Map<String, CacheInfo> caches, > >>> > > > > > > > Map<String, CacheInfo> templates) > >>> > > > > > > > ``` > >>> > > > > > > > I suggest two options for writing arguments. > >>> > > > > > > > > >>> > > > > > > > If arguments are placed in a line before the page > delimiter. > >>> > > > > > > > ```java > >>> > > > > > > > public void foo(Object obj1, Object obj2, Object obj3 , > ... > >>> , > >>> > > > > > > > >>> > > > > > > >>> > > > > > >>> > > > > >>> > > > Object > >>> > > > > > > objN){ > >>> > > > > > > > .... > >>> > > > > > > > } > >>> > > > > > > > ``` > >>> > > > > > > > If the arguments are not placed in the line before the > page > >>> > > > > > > > >>> > > > > > > >>> > > > > > >>> > > > > delimiter. > >>> > > > > > > > > >>> > > > > > > > ```java > >>> > > > > > > > public void foo( > >>> > > > > > > > Object obj1, > >>> > > > > > > > Object obj2, > >>> > > > > > > > Object obj3, > >>> > > > > > > > ... , > >>> > > > > > > > Object objN > >>> > > > > > > > ){ > >>> > > > > > > > .... > >>> > > > > > > > } > >>> > > > > > > > ``` > >>> > > > > > > > In my personal experience, the last example is much > easier > >>> to > >>> > > > > > > > >>> > > > > > > >>> > > > > > >>> > > > > >>> > > > >>> > > merge > >>> > > > > if > >>> > > > > > > > method arguments were changed. > >>> > > > > > > > >>> > > > > > > >>> > > > > > >>> > > > > >>> > > > >>> > > >>> > > >>> > >>> > > > > -- > Best Regards, Vyacheslav D. >
