[jira] [Commented] (MNGSITE-385) Code style should mention "else" style
[
https://issues.apache.org/jira/browse/MNGSITE-385?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16996711#comment-16996711
]
Karl Heinz Marbaise commented on MNGSITE-385:
-
Hi [~bmarwell] of course we should make it easier for committers or
contributors ..I haven't taken this as complain ...
> Code style should mention "else" style
> --
>
> Key: MNGSITE-385
> URL: https://issues.apache.org/jira/browse/MNGSITE-385
> Project: Maven Project Web Site
> Issue Type: Task
>Reporter: Benjamin Marwell
>Assignee: Elliotte Rusty Harold
>Priority: Major
>
> Hello,
> the "code style page" at
> [https://maven.apache.org/developers/conventions/code.html] should mention
> the code style associated with the {{else}} keyword.
> Example: I created my first PR using guard statements and kept the main code
> on the lowest possible indentation level. Only the reviwers told me that this
> is uncommon in apache projects and not wanted. While I do not go with "it's
> clearer to read", it is more common to read if you are used to it not to see
> any code without indentation.
> Here's my PR with the discussion:
> [https://github.com/apache/maven-checkstyle-plugin/pull/17.] You can clearly
> see how I struggled and was suprised with the comments, as checkstyle did
> went through anyway.
> That said, if this is what is wanted, this should be mentioned on the code
> conventions page as well. Maybe even add a checkstyle rule ("no if containing
> break/return" and "no if without else"). While I myself prefer to use guard
> statements, it seems uncommon for apache projects.
> Please update the documentation rules accordingly.
>
--
This message was sent by Atlassian Jira
(v8.3.4#803005)
[jira] [Commented] (MNGSITE-385) Code style should mention "else" style
[
https://issues.apache.org/jira/browse/MNGSITE-385?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16996709#comment-16996709
]
Benjamin Marwell commented on MNGSITE-385:
--
Hello KH,
I had done that. Checkstyle went through, etc.
It's just... Unwritten but enforced rules are impossible to know when you are a
first time committer.
Looking at the code I of course saw the use of else, but I did not thick it was
necessary. That's all to it.
I reformatted it and now it's fine. I just want to make it easier to look such
things up or for other be committers. That's all there is to it. That's not
complaining, just agreement and a suggestion.
> Code style should mention "else" style
> --
>
> Key: MNGSITE-385
> URL: https://issues.apache.org/jira/browse/MNGSITE-385
> Project: Maven Project Web Site
> Issue Type: Task
>Reporter: Benjamin Marwell
>Assignee: Elliotte Rusty Harold
>Priority: Major
>
> Hello,
> the "code style page" at
> [https://maven.apache.org/developers/conventions/code.html] should mention
> the code style associated with the {{else}} keyword.
> Example: I created my first PR using guard statements and kept the main code
> on the lowest possible indentation level. Only the reviwers told me that this
> is uncommon in apache projects and not wanted. While I do not go with "it's
> clearer to read", it is more common to read if you are used to it not to see
> any code without indentation.
> Here's my PR with the discussion:
> [https://github.com/apache/maven-checkstyle-plugin/pull/17.] You can clearly
> see how I struggled and was suprised with the comments, as checkstyle did
> went through anyway.
> That said, if this is what is wanted, this should be mentioned on the code
> conventions page as well. Maybe even add a checkstyle rule ("no if containing
> break/return" and "no if without else"). While I myself prefer to use guard
> statements, it seems uncommon for apache projects.
> Please update the documentation rules accordingly.
>
--
This message was sent by Atlassian Jira
(v8.3.4#803005)
[jira] [Commented] (MNGSITE-385) Code style should mention "else" style
[
https://issues.apache.org/jira/browse/MNGSITE-385?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16996706#comment-16996706
]
Karl Heinz Marbaise commented on MNGSITE-385:
-
[~bmarwell] Please import the given code style into your IDE reformat your PR.
Furthermore if you take a look into the rest of the code base you might see the
differences
> Code style should mention "else" style
> --
>
> Key: MNGSITE-385
> URL: https://issues.apache.org/jira/browse/MNGSITE-385
> Project: Maven Project Web Site
> Issue Type: Task
>Reporter: Benjamin Marwell
>Assignee: Elliotte Rusty Harold
>Priority: Major
>
> Hello,
> the "code style page" at
> [https://maven.apache.org/developers/conventions/code.html] should mention
> the code style associated with the {{else}} keyword.
> Example: I created my first PR using guard statements and kept the main code
> on the lowest possible indentation level. Only the reviwers told me that this
> is uncommon in apache projects and not wanted. While I do not go with "it's
> clearer to read", it is more common to read if you are used to it not to see
> any code without indentation.
> Here's my PR with the discussion:
> [https://github.com/apache/maven-checkstyle-plugin/pull/17.] You can clearly
> see how I struggled and was suprised with the comments, as checkstyle did
> went through anyway.
> That said, if this is what is wanted, this should be mentioned on the code
> conventions page as well. Maybe even add a checkstyle rule ("no if containing
> break/return" and "no if without else"). While I myself prefer to use guard
> statements, it seems uncommon for apache projects.
> Please update the documentation rules accordingly.
>
--
This message was sent by Atlassian Jira
(v8.3.4#803005)
[jira] [Commented] (MNGSITE-385) Code style should mention "else" style
[
https://issues.apache.org/jira/browse/MNGSITE-385?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16996704#comment-16996704
]
Benjamin Marwell commented on MNGSITE-385:
--
That's sad. My first few PRs got rejected because I used a guard statement with
return instead of no return and else.
That's a surprising rule of it is not documented, and somewhat discouraging.
But well, then.
> Code style should mention "else" style
> --
>
> Key: MNGSITE-385
> URL: https://issues.apache.org/jira/browse/MNGSITE-385
> Project: Maven Project Web Site
> Issue Type: Task
>Reporter: Benjamin Marwell
>Assignee: Elliotte Rusty Harold
>Priority: Major
>
> Hello,
> the "code style page" at
> [https://maven.apache.org/developers/conventions/code.html] should mention
> the code style associated with the {{else}} keyword.
> Example: I created my first PR using guard statements and kept the main code
> on the lowest possible indentation level. Only the reviwers told me that this
> is uncommon in apache projects and not wanted. While I do not go with "it's
> clearer to read", it is more common to read if you are used to it not to see
> any code without indentation.
> Here's my PR with the discussion:
> [https://github.com/apache/maven-checkstyle-plugin/pull/17.] You can clearly
> see how I struggled and was suprised with the comments, as checkstyle did
> went through anyway.
> That said, if this is what is wanted, this should be mentioned on the code
> conventions page as well. Maybe even add a checkstyle rule ("no if containing
> break/return" and "no if without else"). While I myself prefer to use guard
> statements, it seems uncommon for apache projects.
> Please update the documentation rules accordingly.
>
--
This message was sent by Atlassian Jira
(v8.3.4#803005)
[jira] [Commented] (MNGSITE-385) Code style should mention "else" style
[
https://issues.apache.org/jira/browse/MNGSITE-385?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16996702#comment-16996702
]
Elliotte Rusty Harold commented on MNGSITE-385:
---
Perhaps Maven might adopt some other project's code style rules one day or
include basic references to Effective Java or the Oracle Java guidelines, but I
don't see us spending the effort to specify everything ourselves at this level
of detail.
> Code style should mention "else" style
> --
>
> Key: MNGSITE-385
> URL: https://issues.apache.org/jira/browse/MNGSITE-385
> Project: Maven Project Web Site
> Issue Type: Task
>Reporter: Benjamin Marwell
>Priority: Major
>
> Hello,
> the "code style page" at
> [https://maven.apache.org/developers/conventions/code.html] should mention
> the code style associated with the {{else}} keyword.
> Example: I created my first PR using guard statements and kept the main code
> on the lowest possible indentation level. Only the reviwers told me that this
> is uncommon in apache projects and not wanted. While I do not go with "it's
> clearer to read", it is more common to read if you are used to it not to see
> any code without indentation.
> Here's my PR with the discussion:
> [https://github.com/apache/maven-checkstyle-plugin/pull/17.] You can clearly
> see how I struggled and was suprised with the comments, as checkstyle did
> went through anyway.
> That said, if this is what is wanted, this should be mentioned on the code
> conventions page as well. Maybe even add a checkstyle rule ("no if containing
> break/return" and "no if without else"). While I myself prefer to use guard
> statements, it seems uncommon for apache projects.
> Please update the documentation rules accordingly.
>
--
This message was sent by Atlassian Jira
(v8.3.4#803005)
[jira] [Commented] (MNGSITE-385) Code style should mention "else" style
[
https://issues.apache.org/jira/browse/MNGSITE-385?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16996453#comment-16996453
]
Michael Osipov commented on MNGSITE-385:
Does it work with Maven from master?
> Code style should mention "else" style
> --
>
> Key: MNGSITE-385
> URL: https://issues.apache.org/jira/browse/MNGSITE-385
> Project: Maven Project Web Site
> Issue Type: Task
>Reporter: Benjamin Marwell
>Priority: Major
>
> Hello,
> the "code style page" at
> [https://maven.apache.org/developers/conventions/code.html] should mention
> the code style associated with the {{else}} keyword.
> Example: I created my first PR using guard statements and kept the main code
> on the lowest possible indentation level. Only the reviwers told me that this
> is uncommon in apache projects and not wanted. While I do not go with "it's
> clearer to read", it is more common to read if you are used to it not to see
> any code without indentation.
> Here's my PR with the discussion:
> [https://github.com/apache/maven-checkstyle-plugin/pull/17.] You can clearly
> see how I struggled and was suprised with the comments, as checkstyle did
> went through anyway.
> That said, if this is what is wanted, this should be mentioned on the code
> conventions page as well. Maybe even add a checkstyle rule ("no if containing
> break/return" and "no if without else"). While I myself prefer to use guard
> statements, it seems uncommon for apache projects.
> Please update the documentation rules accordingly.
>
--
This message was sent by Atlassian Jira
(v8.3.4#803005)
