I agree with you on the second one that it seems more like a preference.
The first one is indeed a rule and I appreciate it that you want to add a
rule and fix any places where we already fail it.
I think the more we automate such kind of things the better.
Thanks again for bringing it in.
Best regards,
Ekaterina
On Wed, 15 Jan 2025 at 18:50, Bernardo Botella <conta...@bernardobotella.com>
wrote:
> Hi everyone!
>
> I wanted to raise a question about code style for the project. I've been
> receiving some feedback on PRs about the need to:
> - Have curly braces start on a new line
> - Remove curly braces if the condition or loop has only one expression
>
> Taking a look at the official Code Style stated in the web, I read that:
> "{ and } are placed on a new line except when empty or opening a
> multi-line lambda expression. Braces may be elided to a depth of one if the
> condition or loop guards a single expression."
>
> Which addresses the first type of comments I mentioned (curly braces
> starting in a new line), but leaves open the second type of comments
> (remove not needed curly braces).
>
> But, when looking at the checkstyle.xml, I don't see any rule enforcing
> any of those two types of comments.
>
> I believe checkstyle.xml should be our contract, so I'm proposing here:
>
> For "curly braces starting in a new line" rule, add something like what we
> already have on Sidecar and Analytics projects:
> <module name="LeftCurly">
> <!-- Checks for placement of the left curly brace ('{'). -->
> <property name="option" value="nl"/>
> ...
> </module>
>
> That way, we can fail fast and not worry about those comments on PRs. This
> of course may be painful, as we probably will have to fix a bunch of
> wrongly placed brackets all over the place.
>
> If there are no concerns here, I'll be more than happy to bite the bullet
> and add a patch for this.
>
>
>
> For "remove not needed curly braces", I understand that it tends to be the
> preference on the code, so we either modify the documentation and add a
> rule for that on the checkstyle.xml, or we are fine with that style and
> there is no need to remove them on patches.
>
> I wanted to hear the thoughts on the community for this one. My preference
> is to always use brackets, but that's just a preference, so it's perfectly
> fine not to enforce it and leave the documentation as is.
>
> Thanks everyone!
> Bernardo
