[VOTE] [CALCITE-490] Remove "End File.java" comments from the end of files

[Vladimir Sitnikov <si...@gmail.com>]

Hi,

Could we have a vote on coding style regarding "// End File.java" in
Calcite source files?
"//End..." comments clutter Git history, they consume screen space, and
they slow down the development by causing Checkstyle violations.

Could we vote on the style regarding "//End File.java"

[ ] +1, remove the ending "// End File.java" comment, ensure files end with
single blank line
[ ] -1, keep "// End File.java" since ...

> https://www.apache.org/foundation/voting.html#votes-on-code-modification
> Whole numbers are recommended for this type of vote, as the opinion being
expressed is Boolean: 'I approve/do not approve of this change.'
>...
> A veto without a justification is invalid and has no weight.


Note back in 2014 Julian did mention 5 thoughts which are questionable.

Julian>And it helps ensure that the file ends with a line ending.

This is just false. The comment line has nothing to di with line ending.
A rule of "file must end with a single blank line" can be verified without
"dedicated comment line".

Julian>It indicates the name of the file (useful if the file has been
renamed).

If a file is renamed, then ending comment HAS to be updated otherwise
Checkstyle fails the build.
In other words, comment always matches "current" file name, thus it cannot
help to detect renames.

Julian>The comment makes it easy to see that you are looking at the last
page of a file in an editor.

This is true, however it does sound like a very rare use-case for me.
Editors often show file name on their own (tab name, application title,
navigation gutter).
Of course, certain UI elements in the IDE can be disabled, however I sill
think IDE is much better at telling you which file you are looking at than
a comment in the very last line of the file.

If we follow that logic, should we add "filename comment" after each 25
lines so one always knows which file is open? That is just non-sense.

Julian>It verifies that the file has not been truncated

If the file is truncated, the compiler will bail out. Unit test could
detect that as well.
On the other hand, "ending comment" does not protect from accidental cut
from a middle of the file.

Julian>and provides a buffer against truncation issues.

I don't get it, however it mentions truncation, thus I assume that is not a
valid justification. Truncation is to be managed by Git / code review /
compiler / test suite.

Vladimir

Re: [VOTE] [CALCITE-490] Remove "End File.java" comments from the end of files

[Michael Mior <mm...@apache.org>]

+0 I've never found these comments caused any problems for me but if the
consensus is that it slows down things for others, I have nothing against
removing them.

--
Michael Mior
mmior@apache.org


Le lun. 10 sept. 2018 à 04:49, Vladimir Sitnikov <
sitnikov.vladimir@gmail.com> a écrit :

> Hi,
>
> Could we have a vote on coding style regarding "// End File.java" in
> Calcite source files?
> "//End..." comments clutter Git history, they consume screen space, and
> they slow down the development by causing Checkstyle violations.
>
> Could we vote on the style regarding "//End File.java"
>
> [ ] +1, remove the ending "// End File.java" comment, ensure files end with
> single blank line
> [ ] -1, keep "// End File.java" since ...
>
> > https://www.apache.org/foundation/voting.html#votes-on-code-modification
> > Whole numbers are recommended for this type of vote, as the opinion being
> expressed is Boolean: 'I approve/do not approve of this change.'
> >...
> > A veto without a justification is invalid and has no weight.
>
>
> Note back in 2014 Julian did mention 5 thoughts which are questionable.
>
> Julian>And it helps ensure that the file ends with a line ending.
>
> This is just false. The comment line has nothing to di with line ending.
> A rule of "file must end with a single blank line" can be verified without
> "dedicated comment line".
>
> Julian>It indicates the name of the file (useful if the file has been
> renamed).
>
> If a file is renamed, then ending comment HAS to be updated otherwise
> Checkstyle fails the build.
> In other words, comment always matches "current" file name, thus it cannot
> help to detect renames.
>
> Julian>The comment makes it easy to see that you are looking at the last
> page of a file in an editor.
>
> This is true, however it does sound like a very rare use-case for me.
> Editors often show file name on their own (tab name, application title,
> navigation gutter).
> Of course, certain UI elements in the IDE can be disabled, however I sill
> think IDE is much better at telling you which file you are looking at than
> a comment in the very last line of the file.
>
> If we follow that logic, should we add "filename comment" after each 25
> lines so one always knows which file is open? That is just non-sense.
>
> Julian>It verifies that the file has not been truncated
>
> If the file is truncated, the compiler will bail out. Unit test could
> detect that as well.
> On the other hand, "ending comment" does not protect from accidental cut
> from a middle of the file.
>
> Julian>and provides a buffer against truncation issues.
>
> I don't get it, however it mentions truncation, thus I assume that is not a
> valid justification. Truncation is to be managed by Git / code review /
> compiler / test suite.
>
> Vladimir
>

Re: [VOTE] [CALCITE-490] Remove "End File.java" comments from the end of files

[Julian Hyde <jh...@apache.org>]

-0

I don’t think it’s worth getting into debates about coding style.

> On Sep 10, 2018, at 1:12 PM, Josh Elser <el...@apache.org> wrote:
> 
> -0 I don't see this as a burden as you do, but don't feel strongly either way.
> 
> On 9/10/18 4:49 AM, Vladimir Sitnikov wrote:
>> Hi,
>> Could we have a vote on coding style regarding "// End File.java" in
>> Calcite source files?
>> "//End..." comments clutter Git history, they consume screen space, and
>> they slow down the development by causing Checkstyle violations.
>> Could we vote on the style regarding "//End File.java"
>> [ ] +1, remove the ending "// End File.java" comment, ensure files end with
>> single blank line
>> [ ] -1, keep "// End File.java" since ...
>>> https://www.apache.org/foundation/voting.html#votes-on-code-modification
>>> Whole numbers are recommended for this type of vote, as the opinion being
>> expressed is Boolean: 'I approve/do not approve of this change.'
>>> ...
>>> A veto without a justification is invalid and has no weight.
>> Note back in 2014 Julian did mention 5 thoughts which are questionable.
>> Julian>And it helps ensure that the file ends with a line ending.
>> This is just false. The comment line has nothing to di with line ending.
>> A rule of "file must end with a single blank line" can be verified without
>> "dedicated comment line".
>> Julian>It indicates the name of the file (useful if the file has been
>> renamed).
>> If a file is renamed, then ending comment HAS to be updated otherwise
>> Checkstyle fails the build.
>> In other words, comment always matches "current" file name, thus it cannot
>> help to detect renames.
>> Julian>The comment makes it easy to see that you are looking at the last
>> page of a file in an editor.
>> This is true, however it does sound like a very rare use-case for me.
>> Editors often show file name on their own (tab name, application title,
>> navigation gutter).
>> Of course, certain UI elements in the IDE can be disabled, however I sill
>> think IDE is much better at telling you which file you are looking at than
>> a comment in the very last line of the file.
>> If we follow that logic, should we add "filename comment" after each 25
>> lines so one always knows which file is open? That is just non-sense.
>> Julian>It verifies that the file has not been truncated
>> If the file is truncated, the compiler will bail out. Unit test could
>> detect that as well.
>> On the other hand, "ending comment" does not protect from accidental cut
>> from a middle of the file.
>> Julian>and provides a buffer against truncation issues.
>> I don't get it, however it mentions truncation, thus I assume that is not a
>> valid justification. Truncation is to be managed by Git / code review /
>> compiler / test suite.
>> Vladimir

Re: [VOTE] [CALCITE-490] Remove "End File.java" comments from the end of files

[Vladimir Sitnikov <si...@gmail.com>]

Josh>-0 I don't see this as a burden as you do, but don't feel strongly

Here you go, sir:
https://github.com/apache/calcite/pull/830/commits/73452b9d89f1677d1dcbe0d7d1c642bc9082b828#diff-977410f84ab14e4d3890bd28f221ed8dR359
I've no idea how come I named the class with "Programm" inside, however
after rename I got a checkstyle issue.


Vladimir

Re: [VOTE] [CALCITE-490] Remove "End File.java" comments from the end of files

[Josh Elser <el...@apache.org>]

-0 I don't see this as a burden as you do, but don't feel strongly 
either way.

On 9/10/18 4:49 AM, Vladimir Sitnikov wrote:
> Hi,
> 
> Could we have a vote on coding style regarding "// End File.java" in
> Calcite source files?
> "//End..." comments clutter Git history, they consume screen space, and
> they slow down the development by causing Checkstyle violations.
> 
> Could we vote on the style regarding "//End File.java"
> 
> [ ] +1, remove the ending "// End File.java" comment, ensure files end with
> single blank line
> [ ] -1, keep "// End File.java" since ...
> 
>> https://www.apache.org/foundation/voting.html#votes-on-code-modification
>> Whole numbers are recommended for this type of vote, as the opinion being
> expressed is Boolean: 'I approve/do not approve of this change.'
>> ...
>> A veto without a justification is invalid and has no weight.
> 
> 
> Note back in 2014 Julian did mention 5 thoughts which are questionable.
> 
> Julian>And it helps ensure that the file ends with a line ending.
> 
> This is just false. The comment line has nothing to di with line ending.
> A rule of "file must end with a single blank line" can be verified without
> "dedicated comment line".
> 
> Julian>It indicates the name of the file (useful if the file has been
> renamed).
> 
> If a file is renamed, then ending comment HAS to be updated otherwise
> Checkstyle fails the build.
> In other words, comment always matches "current" file name, thus it cannot
> help to detect renames.
> 
> Julian>The comment makes it easy to see that you are looking at the last
> page of a file in an editor.
> 
> This is true, however it does sound like a very rare use-case for me.
> Editors often show file name on their own (tab name, application title,
> navigation gutter).
> Of course, certain UI elements in the IDE can be disabled, however I sill
> think IDE is much better at telling you which file you are looking at than
> a comment in the very last line of the file.
> 
> If we follow that logic, should we add "filename comment" after each 25
> lines so one always knows which file is open? That is just non-sense.
> 
> Julian>It verifies that the file has not been truncated
> 
> If the file is truncated, the compiler will bail out. Unit test could
> detect that as well.
> On the other hand, "ending comment" does not protect from accidental cut
> from a middle of the file.
> 
> Julian>and provides a buffer against truncation issues.
> 
> I don't get it, however it mentions truncation, thus I assume that is not a
> valid justification. Truncation is to be managed by Git / code review /
> compiler / test suite.
> 
> Vladimir
> 

Re: [VOTE] [CALCITE-490] Remove "End File.java" comments from the end of files

[Christian Beikov <ch...@gmail.com>]

+1

Am 10.09.2018 um 10:49 schrieb Vladimir Sitnikov:
> Hi,
>
> Could we have a vote on coding style regarding "// End File.java" in
> Calcite source files?
> "//End..." comments clutter Git history, they consume screen space, and
> they slow down the development by causing Checkstyle violations.
>
> Could we vote on the style regarding "//End File.java"
>
> [ ] +1, remove the ending "// End File.java" comment, ensure files end with
> single blank line
> [ ] -1, keep "// End File.java" since ...
>
>> https://www.apache.org/foundation/voting.html#votes-on-code-modification
>> Whole numbers are recommended for this type of vote, as the opinion being
> expressed is Boolean: 'I approve/do not approve of this change.'
>> ...
>> A veto without a justification is invalid and has no weight.
>
> Note back in 2014 Julian did mention 5 thoughts which are questionable.
>
> Julian>And it helps ensure that the file ends with a line ending.
>
> This is just false. The comment line has nothing to di with line ending.
> A rule of "file must end with a single blank line" can be verified without
> "dedicated comment line".
>
> Julian>It indicates the name of the file (useful if the file has been
> renamed).
>
> If a file is renamed, then ending comment HAS to be updated otherwise
> Checkstyle fails the build.
> In other words, comment always matches "current" file name, thus it cannot
> help to detect renames.
>
> Julian>The comment makes it easy to see that you are looking at the last
> page of a file in an editor.
>
> This is true, however it does sound like a very rare use-case for me.
> Editors often show file name on their own (tab name, application title,
> navigation gutter).
> Of course, certain UI elements in the IDE can be disabled, however I sill
> think IDE is much better at telling you which file you are looking at than
> a comment in the very last line of the file.
>
> If we follow that logic, should we add "filename comment" after each 25
> lines so one always knows which file is open? That is just non-sense.
>
> Julian>It verifies that the file has not been truncated
>
> If the file is truncated, the compiler will bail out. Unit test could
> detect that as well.
> On the other hand, "ending comment" does not protect from accidental cut
> from a middle of the file.
>
> Julian>and provides a buffer against truncation issues.
>
> I don't get it, however it mentions truncation, thus I assume that is not a
> valid justification. Truncation is to be managed by Git / code review /
> compiler / test suite.
>
> Vladimir
>