search

ComputationalRadiationPhysics / contributing

:books: guidelines and styles to contribute to our projects
Creative Commons Attribution 4.0 International
6 stars 10 forks source link

[clang-tidy] Comment check #45

Closed j-stephan closed 3 years ago

j-stephan commented 3 years ago

This PR contains the check related to comments. Voting will end on 17 November 2020, 18:00 HZDR time.

Vote

Enable Disable
0 1
bernhardmgruber commented 3 years ago
Enable Disable
1 1

This one is important. If a codebase gets older, you will have many TODOs in there and your git/SVN log is not helping you with who wrote it, because the code around got refactored, formattet etc. So adding a username to a TODO gives you a chance to still find that person, if the comment was important.

SimeonEhrig commented 3 years ago

Git blame will show you the author of the comment.

bernhardmgruber commented 3 years ago

Git blame will show you the author of the comment.

No, it will show you the last person who edited the line.

So if I wrote:

func(a, b); // TODO: this might be slow

And you refactor it to, maybe even as part of an automatic rename:

Func(a, b); // TODO: this might be slow

Then git blame will give me your name and not the TODO author's.

j-stephan commented 3 years ago

Requesting a tie break here. @psychocoderHPC @sbastrakov @SimeonEhrig @sliwowitz or anyone else who has feelings about this...

j-stephan commented 3 years ago

Ok, I'll do it myself. I actually now think that this is quite useful. Final result:

Enable Disable
2 0
SimeonEhrig commented 3 years ago

Git blame will show you the author of the comment.

It's also possible to iterate throw the history with git blame and look after the original author of the TODO, but I'm fine, if it is forced by clang-tidy. The consistence check is the time consuming stuff, not writing the comment ;-)

Enable Disable
3 0