Hi, I filed the ticket https://issues.apache.org/jira/browse/IGNITE-8512
I'll do this when I have enough time if it won't be done earlier. On Mon, May 14, 2018 at 8:46 PM, Dmitry Pavlov <dpavlov....@gmail.com> wrote: > Hi Vyacheslav, > > plugin was published in AI wiki, feel free to create ticket to add method > code style check. > > Actually I'm not sure it is easy to validate code style in plugin, but I > guess you know it better. > > 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. >> -- Best Regards, Vyacheslav D.
