Hey Denis,
Yep, I've been using the Ignite code style for IntelliJ since the
beginning.
It's ok for indentation, whitespacing, line breaks, etc. But things too
specific like abbreviations, string outputs, Javadoc conventions like using
{@code ...}, etc. are not covered.
checkstyle is a potential solution to automate the audit. We'll definitely
need to implement custom checks, but it's worth it. Earlier I saw a comment
in a ticket from another user asking for a Sonar-like report of their
mistakes... So at least I'm not alone in my frustration!
Regards,
*Raúl Kripalani*
PMC & Committer @ Apache Ignite, Apache Camel | Integration, Big Data and
Messaging Engineer
http://about.me/raulkripalani | http://www.linkedin.com/in/raulkripalani
http://blog.raulkr.net | twitter: @raulvk
On Thu, Oct 22, 2015 at 8:59 PM, Denis Magda <dma...@gridgain.com> wrote:
> Raul,
>
> It wasn't easy for me to get into the way of following Ignite coding
> guidelines. So you're not alone :)
>
> However one day I found "ignite/idea/ignite_codeStyle.xml" file and set it
> in Intellij IDEA settings. Since that time coding guidelines has never
> worried me again. Probably this will help you as well.
>
>
> --
> Denis
>
>
> On 10/22/2015 10:50 PM, Dmitriy Setrakyan wrote:
>
>> Raul,
>>
>> Thanks for your contribution!
>>
>> I want to address the coding guidelines comments, as they keep coming up
>> again and again. I do agree that coding guidelines may not be ideal, and
>> may seem redundant at times. However, they make Ignite code look
>> consistent. If you try to change them now, even if it seems in a better
>> direction, the overall impact on the code will be negative, as the old
>> code
>> will remain inconsistent with new one.
>>
>> I am also noticing that every discussion about coding guidelines is about
>> a
>> matter of taste. We will never all agree on the taste, so my preference
>> would be just to follow the guidelines as they are.
>>
>> I also think that we should relax the coding guidelines for the
>> contributors (non-committers) as long as they get fixed by the committer
>> who does the final merge. This will make contributions easier (I will
>> start
>> a separate thread about it).
>>
>> D.
>>
>> On Thu, Oct 22, 2015 at 12:22 PM, Raul Kripalani <ra...@apache.org>
>> wrote:
>>
>> Hi Yakov,
>>>
>>> Thanks for the review.
>>>
>>> Comments inline.
>>>
>>> Aside from them: being honest, do you think Javadoc like the following
>>> snippets are useful and provide any value? More so in a test case...
>>>
>>> Snippet #1:
>>>
>>> /**
>>> * @param topics Topics.
>>> * @param fromIdx From index.
>>> * @param cnt Count.
>>> * @param singleMsg Single message flag.
>>> * @throws MqttException If failed.
>>> */
>>>
>>> Snippet #2:
>>>
>>> /**
>>> * @throws Exception If failed.
>>> */
>>>
>>> To me, they are redundant. And I'm concerned about imposing superfluous
>>> Javadoc "just because", or due to aesthetics, rather than value.
>>>
>>> Regards,
>>>
>>> *Raúl Kripalani*
>>> PMC & Committer @ Apache Ignite, Apache Camel | Integration, Big Data and
>>> Messaging Engineer
>>> http://about.me/raulkripalani | http://www.linkedin.com/in/raulkripalani
>>> http://blog.raulkr.net | twitter: @raulvk
>>>
>>> On Thu, Oct 22, 2015 at 11:52 AM, Yakov Zhdanov <yzhda...@apache.org>
>>> wrote:
>>>
>>> Hi Raul!
>>>>
>>>> Thank you very much for addressing my comments!
>>>>
>>>> I like the code however there are still a couple of points (I committed
>>>> everything to ignite-1747):
>>>>
>>>> 1. Please take a look at
>>>>
>>>>
>>>>
>>> https://cwiki.apache.org/confluence/display/IGNITE/Coding+Guidelines#CodingGuidelines-StringOutput
>>>
>>>
>>> Yeah, I'm familiar with this rule and I thought I was complying with it.
>>> Could you give me an example where the code is not compliant?
>>>
>>>
>>> 2. org.apache.ignite.stream.mqtt.MqttStreamer#connected - volatile write
>>>> then read of most likely the same value - I changed it to return true
>>>> literal.
>>>> connected = true;
>>>> return connected;
>>>>
>>>> Agree.
>>>
>>>
>>> 3. blockUntilConnected - I would name this property syncConn
>>>>
>>>> Why? To me "blockUntilConnected" is clearer and more explicit than
>>> "syncConn", which sounds mystic to an ordinary user who's not familiar
>>> with
>>> the community's abbreviation table. It's ok for a private member, but not
>>> for a property. In fact, syncConn requires the user to have the Javadoc
>>> handy, blockUntilConnected doesn't. So I prefer the latter.
>>>
>>>
>>> 4. Abbreviation rules violations - connected - conn, executor - exec,
>>>> values - vals, count - cnt, message - msg
>>>>
>>>> Ok, this is a rule... yeah. But you can imagine it's extremely hard to
>>> memorise someone else's glossary. In my opinion, abbreviations should be
>>> used only when needed, not by default. An abbreviation subtracts
>>> readability for the general user – long-term Ignite developers are
>>> probably
>>> used to these by now.
>>>
>>>
>>> 5. What is the point of "connected" member? Client has "isConnected()"?
>>>>
>>> Is
>>>
>>>> it similar to what you want to achieve?
>>>>
>>>> I'll look at the implementation of MqttClient.isConnected(), but off the
>>> top of my head, it seems it could be replaced, yes.
>>>
>>>
>>> 6. Race on stop - 1 thread calls stop and successfully exits the method,
>>>> retrier is not stopped (awaitTermination has not been called - should it
>>>> be?) and can connect client back - is this the case?
>>>>
>>>> I'll have a look.
>>>
>>>
>>> 7. I think we use {@code } instead of <tt>...
>>>>
>>>> Correct. Thanks.
>>>
>>> On more point community should agree on - imports ordering and
>>> grouping. I
>>>
>>>> will post another email.
>>>>
>>>> This was already added a few weeks ago in the Coding rules.
>>>
>>>
>>> --Yakov
>>>>
>>>> 2015-10-20 20:39 GMT+03:00 Raul Kripalani <ra...@apache.org>:
>>>>
>>>> Hi Yakov,
>>>>>
>>>>> I've had a few free cycles to fix the code style and Javadoc issues you
>>>>> reported in the MQTT Streamer and I've pushed my changes to branch
>>>>> ignite-1747.
>>>>>
>>>>> Would you mind having a look to see if it adapts better now?
>>>>>
>>>>> Thanks,
>>>>>
>>>>> *Raúl Kripalani*
>>>>> PMC & Committer @ Apache Ignite, Apache Camel | Integration, Big Data
>>>>>
>>>> and
>>>
>>>> Messaging Engineer
>>>>> http://about.me/raulkripalani |
>>>>>
>>>> http://www.linkedin.com/in/raulkripalani
>>>
>>>> http://blog.raulkr.net | twitter: @raulvk
>>>>>
>>>>>
>
