I'll leave Julian to clarify whether or not the -1 was intended to be a veto, but as you noted, Julian expressed some concern in the PR and the associated JIRA. If we are voting, I'm currently -0 because I haven't found checkstyle violations to be significant problem for me and I don't like the idea of losing checks which are currently in place. Since you already have a PR open to HydromaticFileSetCheck support checkstyle 8, it seems like the changes aren't complex. Given that repo is also Apache licensed, wee could always pull the relevant code into Calcite since it's fairly small. -- Michael Mior [email protected]
Le mer. 4 déc. 2019 à 15:04, Vladimir Sitnikov <[email protected]> a écrit : > > Michael>I didn't interpret Jullian's -1 as a veto > > > https://www.apache.org/foundation/voting.html#Veto > > A code-modification proposal may be stopped dead in its tracks by a -1 > vote by a qualified voter. > > This constitutes a veto, and it cannot be overruled nor overridden by > anyone. > > Vetos stand until and unless withdrawn by their casters. > > Apparently that is a veto. What else could it be? However, it is "invalid > and has no weight". > > I'm open to new opinions, however, I'm sure I have provided enough reasons > to just commit the PR and move forward. > > Note: the only reason I started this discussion is I saw Julian's comments > in PR and in the JIRA, so I wanted to gather opinions. > Frankly speaking, I see nothing to discuss here. > > Here's a recent (created 25m ago) PR by Andrei: > https://github.com/apache/calcite/pull/1632 > Apparently, it failed with > [ant:checkstyle] [ERROR] > D:\a\calcite\calcite\core\src\main\java\org\apache\calcite\util\Sources.java:108: > Open parentheses exceed closes by 2 or more [HydromaticFileSet] > https://github.com/apache/calcite/pull/1632/checks#step:4:309 > > Here's how the new error would look like after HydromaticFileSetCheck is > dropped: > > > The following files had format violations: > core/src/main/java/org/apache/calcite/util/Sources.java > @@ -105,7 +105,8 @@ > ····} > > ····private·UnsupportedOperationException·unsupported()·{ > > -······return·new·UnsupportedOperationException(String.format(Locale.ROOT, > +······return·new·UnsupportedOperationException( > +··········String.format(Locale.ROOT, > ··········"Invalid·operation·for·'%s'·protocol",·protocol())); > ····} > > I find this error to be way better than "by 2 or more". > > Vladimir
