You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@iceberg.apache.org by "RussellSpitzer (via GitHub)" <gi...@apache.org> on 2026/02/23 21:28:17 UTC
[PR] Checkstyle, Spotless: Force newline after closing brace [iceberg]
RussellSpitzer opened a new pull request, #15422:
URL: https://github.com/apache/iceberg/pull/15422
Uses the following Regex to find violations
(?m)(^[ \t]*\}[ \t]*\n)(?![ \t]*\n)(?![ \t]*\})(?![ \t]*\z) -----------------------====================================
------------- Match Section ----------------
(?m) — Multiline mode. Makes ^ match the start of every line, not just the start of the file.
( — Opens capture group $1 (used by the spotless replacement).
^[ \t]*\} — Match any amount of white space ending in a brace character.
Excludes Javadoc (* {@link Foo}), comments (// something}), and inline expressions (new String[] {...}).
[ \t]*\n — Match optional trailing whitespace after }, then the newline ending that line.
) — Closes capture group $1. Everything above is "the closing brace line".
============= Look Ahead Section ====================
The three negative lookaheads check what comes next:
(?![ \t]*\n) — The next line is NOT all white-space. If there's already a blank line, the rule is already satisfied — don't flag it or add another. (?![ \t]*\}) — The next line must NOT start with another }. Consecutive closing braces (like the end of nested blocks) don't need blank lines between them. (?![ \t]*\z) — The next line is NOT EOF. Don't flag or insert a trailing blank line at the end of a file.
Used Cursor extensively for Regex testing and reviewing the existing codebase to determine exactly what we needed to fix the current setup.
--
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-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] Checkstyle, Spotless: Force newline after closing brace [iceberg]
Posted by "RussellSpitzer (via GitHub)" <gi...@apache.org>.
RussellSpitzer commented on code in PR #15422:
URL: https://github.com/apache/iceberg/pull/15422#discussion_r2848155653
##########
api/src/main/java/org/apache/iceberg/expressions/BoundLiteralPredicate.java:
##########
@@ -114,24 +114,28 @@ public boolean isEquivalentTo(Expression expr) {
// < 6 is equivalent to <= 5
return toLong(literal()) == toLong(other.literal()) + 1L;
}
+
Review Comment:
It's easy enough to add another "not lookahead" for "break;" the question is just how many exceptions do we want to add
--
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-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] Checkstyle, Spotless: Force newline after closing brace [iceberg]
Posted by "RussellSpitzer (via GitHub)" <gi...@apache.org>.
RussellSpitzer commented on PR #15422:
URL: https://github.com/apache/iceberg/pull/15422#issuecomment-3970303231
> Empty lines are a way to structure the code and introduce logical groups. I am not sure you can align that by one rule.
https://iceberg.apache.org/contribute/#block-spacing < We already did :) But I feel ya,
If we can't make it a machine enforced rule, it's not really code style. But maybe i've been brainwashed by the rust and go folks.
--
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-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] Checkstyle, Spotless: Force newline after closing brace [iceberg]
Posted by "RussellSpitzer (via GitHub)" <gi...@apache.org>.
RussellSpitzer commented on code in PR #15422:
URL: https://github.com/apache/iceberg/pull/15422#discussion_r2843183454
##########
api/src/main/java/org/apache/iceberg/expressions/BoundLiteralPredicate.java:
##########
@@ -114,24 +114,28 @@ public boolean isEquivalentTo(Expression expr) {
// < 6 is equivalent to <= 5
return toLong(literal()) == toLong(other.literal()) + 1L;
}
+
Review Comment:
I don't really like how this looks, but having the newline after the "break;" would make the regex look much more complicated ... maybe still worth doing?
--
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-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] Checkstyle, Spotless: Force newline after closing brace [iceberg]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #15422:
URL: https://github.com/apache/iceberg/pull/15422#issuecomment-4151444907
This pull request has been marked as stale due to 30 days of inactivity. It will be closed in 1 week if no further activity occurs. If you think that’s incorrect or this pull request requires a review, please simply write any comment. If closed, you can revive the PR at any time and @mention a reviewer or discuss it on the dev@iceberg.apache.org list. Thank you for your contributions.
--
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-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] Checkstyle, Spotless: Force newline after closing brace [iceberg]
Posted by "aokolnychyi (via GitHub)" <gi...@apache.org>.
aokolnychyi commented on PR #15422:
URL: https://github.com/apache/iceberg/pull/15422#issuecomment-3947642382
I think not using that in certain cases is actually acceptable or even preferable. If the method body is short, you may want to skip the extra empty lines around if blocks.
--
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-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] Checkstyle, Spotless: Force newline after closing brace [iceberg]
Posted by "pvary (via GitHub)" <gi...@apache.org>.
pvary commented on code in PR #15422:
URL: https://github.com/apache/iceberg/pull/15422#discussion_r2845758945
##########
api/src/main/java/org/apache/iceberg/expressions/BoundLiteralPredicate.java:
##########
@@ -114,24 +114,28 @@ public boolean isEquivalentTo(Expression expr) {
// < 6 is equivalent to <= 5
return toLong(literal()) == toLong(other.literal()) + 1L;
}
+
Review Comment:
Maybe we can move to the new `switch` statements, and add a regexp for `}\ncase\s`?
--
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-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] Checkstyle, Spotless: Force newline after closing brace [iceberg]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] closed pull request #15422: Checkstyle, Spotless: Force newline after closing brace
URL: https://github.com/apache/iceberg/pull/15422
--
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-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] Checkstyle, Spotless: Force newline after closing brace [iceberg]
Posted by "pvary (via GitHub)" <gi...@apache.org>.
pvary commented on PR #15422:
URL: https://github.com/apache/iceberg/pull/15422#issuecomment-3950498813
> > I think not using that in certain cases is actually acceptable or even preferable. If the method body is short, you may want to skip the extra empty lines around if blocks.
>
> Fair enough, currently it's all up to the reviewer though to determine this so it's not great.
I'm really tried of pointing out the new line stuff for every review.
Could we just make this work reasonably well, and do the refactor before a release?
--
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-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] Checkstyle, Spotless: Force newline after closing brace [iceberg]
Posted by "RussellSpitzer (via GitHub)" <gi...@apache.org>.
RussellSpitzer commented on PR #15422:
URL: https://github.com/apache/iceberg/pull/15422#issuecomment-3947700606
> I think not using that in certain cases is actually acceptable or even preferable. If the method body is short, you may want to skip the extra empty lines around if blocks.
Fair enough, currently it's all up to the reviewer though to determine this so it's not great.
--
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-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] Checkstyle, Spotless: Force newline after closing brace [iceberg]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #15422:
URL: https://github.com/apache/iceberg/pull/15422#issuecomment-4189812311
This pull request has been closed due to lack of activity. This is not a judgement on the merit of the PR in any way. It is just a way of keeping the PR queue manageable. If you think that is incorrect, or the pull request requires review, you can revive the PR at any time.
--
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-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org