-
Notifications
You must be signed in to change notification settings - Fork 162
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
kernel: convert many C comments to C++ comments #5271
Conversation
src/costab.c
Outdated
Int rc; // right coset to apply to | ||
Int tc; // temporary coset | ||
Int i; // loop variable | ||
Obj hdTmp; // temporary variable | ||
|
||
/*T 1996/12/03 fceller this should be replaced by 'PlistConv' */ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this one not handled?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Aha, indeed. This is because of the T
. We have to decide how to treat such comments. Presumably could be converted to // TODO:
. Does that seems sensible?
Just to explain, the main part of this (in the first commit) was created using a very simple perl invocation. The idea is to take care of the 95% of "easy" cases in a fully automated way, to make it easy to rebase the PR if needed. Then the remaining cases may need some more manual work. Of course I can add more "automated" changes, but I'd like to keep each separate transformation as simple as possible.
i'm happy for this to be merged as-is, and my comments to be dealt with later, just wanted to raise them as I checked to make sure nothing looked broken. |
Ideally I'd like to get PR #5256 merged first, as it is easier to deal with resulting conflicts in this PR here (as most of the changes can be easily re-created by re-running a script. |
8878d6e
to
5e354ab
Compare
This commit was generated using this command: perl -pi -e 's;/\*[ \t]+(.+[^ \t])[ \t]+\*/[ \t]*$;// \1;g' src/*.* src/hpc/*.*
- convert more C comments to C++ comments - remove empty comments - remove some weird "patterns" at the end of some C++ comments - fix some indentation issues and other minor stuff in a bunch of comments - remove some obsolete comments
5e354ab
to
798a2a4
Compare
This is now good to go from my POV |
Why? For the inline comments deal with in the first commit (made using a perl invocation) the reasoning is this: Adjusting comments like
/* catch infix operations */
or
to always fill exactly the correct number of columns is a major PITA. Failing to shorten such a comment also interacts badly with
clang-format
..For multi-line comments, I usually also find it easier to come up with a uniform styling when using C++ comments; for multiline C comments, we have a plethora of ways to format them. Which of course isn't a serious problem on its own, except it adds mental load ("how should I format this comment??")
Note that this PR deliberately does not touch the big "block" comments of this kind (other than "fixing" some with "obvious" deviations):
I actually also have a script converting those, but this may require more discussion and so I wanted to keep it seperate.