[GitHub] [hbase] ndimiduk commented on pull request #3913: HBASE-26536 Tweak checkstyle LeftCurly config to "nlow"
ndimiduk commented on pull request #3913: URL: https://github.com/apache/hbase/pull/3913#issuecomment-998145153 @Apache9 if you'd like to try out spotless, please open a separate issue for that. I think it looks promising. In the mean time, what's wrong with the single line scoped block as I propose on this change? I find it this change makes code easier to read by reducing the space needed to represent a small, complete expression. It also honors the need for explicit block markers, something that the bare then-statement does not provide. Is there anything problematic about this style that I propose? Thanks. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: issues-unsubscr...@hbase.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [hbase] ndimiduk commented on pull request #3913: HBASE-26536 Tweak checkstyle LeftCurly config to "nlow"
ndimiduk commented on pull request #3913:
URL: https://github.com/apache/hbase/pull/3913#issuecomment-996937315
> For me I do not like to have a left '{' and right '}' on the same line,
unless there is nothing inside the '{}'.
Oh interesting, so you prefer
```java
if (foo == null) return null;
```
over my proposed,
```java
if (foo == null) { return null; }
```
? I think adding the surrounding '{}' makes good from a bad situation.
> So I suggest we just keep the old style, I do not think this is a blocker?
Just update the formatter and do a format.
We have a little bit of everything -- I don't think any of this is a blocker.
> And to end the checkstyle war, do you have any interest to introduce the
spotless plugin to our project? It can format all the java files with a ecplise
formatter file, add headers, and could even format files other than java.
I don't know Spotless... This looks promising. Its maven support is almost
as good as gradle. I don't find IDE support for Spotless. There's a guide on
exporting a spotless config to an eclipse format file, but this won't stay up
to date with changes to the authoritative config source. So what does Spotless
get us that Checkstyle doesn't?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscr...@hbase.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
[GitHub] [hbase] ndimiduk commented on pull request #3913: HBASE-26536 Tweak checkstyle LeftCurly config to "nlow"
ndimiduk commented on pull request #3913: URL: https://github.com/apache/hbase/pull/3913#issuecomment-993063501 The `.editorconfig` has drifted from the checkstyle file, I'm not sure why/how. Maybe a newer version of the IntelliJ or the IntelliJ Checkstyle Plugin have changed how settings are represented. I've pushed a change to editorconfig that goes along with this checkstyle config change. There may be more to it, but this is an obvious difference between what we have and the editorconfig file that is generated after this checkstyle configuration change. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: issues-unsubscr...@hbase.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [hbase] ndimiduk commented on pull request #3913: HBASE-26536 Tweak checkstyle LeftCurly config to "nlow"
ndimiduk commented on pull request #3913: URL: https://github.com/apache/hbase/pull/3913#issuecomment-993021853 Eclipse should be using the formatting configuration that is committed to the project, which was generated from checkstyle config. Actually,let me update both configurations here. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: issues-unsubscr...@hbase.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [hbase] ndimiduk commented on pull request #3913: HBASE-26536 Tweak checkstyle LeftCurly config to "nlow"
ndimiduk commented on pull request #3913: URL: https://github.com/apache/hbase/pull/3913#issuecomment-987009061 > For me I do not think it is a big deal to not allow single line if block, just a reformat is enough? At least I know eclipse by default will format the single line block to multi lines. For me, it's not about writing code, it's about reading code. I find the 1-line version of these statements easier to read and impose less cognitive load overall. As for formatting and IDE, the IDE is configurable (mostly), and is controlled by formatting rules that we control. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: issues-unsubscr...@hbase.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [hbase] ndimiduk commented on pull request #3913: HBASE-26536 Tweak checkstyle LeftCurly config to "nlow"
ndimiduk commented on pull request #3913: URL: https://github.com/apache/hbase/pull/3913#issuecomment-985726228 > I don't have strong opinion on the this single line statement, but I'm wondered what standard we're following? e.g. myself always find [google's java style](https://google.github.io/styleguide/javaguide.html#s4.3-one-statement-per-line) handy, that does not suggest multiple statements in a single line. I don't think we follow a specific style. If you're curious, you could diff the checkstyle provided Google Style definition vs ours and see how close they are. Maybe @busbey knows how we got here. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: issues-unsubscr...@hbase.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [hbase] ndimiduk commented on pull request #3913: HBASE-26536 Tweak checkstyle LeftCurly config to "nlow"
ndimiduk commented on pull request #3913:
URL: https://github.com/apache/hbase/pull/3913#issuecomment-985703248
> What's the problem on the current style check?
Sorry, I thought I left a reply, but apparently it didn't save.
This change allows for more compact single-line statements, such as,
```java
if (foo == null) { return null; }
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscr...@hbase.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
