-
Notifications
You must be signed in to change notification settings - Fork 32
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
Turn on clang-tidy check for function naming and change function names to pass check #310
Turn on clang-tidy check for function naming and change function names to pass check #310
Conversation
…on readability-identifier-naming check
…on readability-identifier-naming check
All the MHD and testing stuff looks good to me. |
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.
I think the disk_ICs function renaming is good, but there are a couple library function names that need to be changed back.
ddi_(sqr(double(n[0] - 1) / (hi[0] - lo[0])) / 6.0), | ||
ddj_(sqr(double(n[1] - 1) / (hi[1] - lo[1])) / 6.0), | ||
ddk_(sqr(double(n[2] - 1) / (hi[2] - lo[2])) / 6.0), | ||
ddi_(Sqr(double(n[0] - 1) / (hi[0] - lo[0])) / 6.0), |
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.
sqrt
is a library call, so this name change will break the code.
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.
Maybe I'm misunderstanding something, but I thought those were calls to an inline function defined in line 7 of this file that returns the square of the argument?
I think this one is good to go once it's merged with the updated version of dev. |
For some reason that's not currently clear to me, clang-tidy only catches some instances of function names not conforming to the
Upper_Snake_Case
naming convention that we've chosen. I've corrected all of the functions that the check was failing for (and others that I happened to catch while doing this) and the check now passes.For the most part, it was straightforward to choose new versions of the function names to conform to our naming standard, but there are a few cases that I could use a second opinion on. In particular, there were a few function names in models/disk_ICs.cpp that I didn't understand the meaning of, so the new names that I gave them might not be the best.