-
Notifications
You must be signed in to change notification settings - Fork 192
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
Organizational changes to shared CCE code #6354
Organizational changes to shared CCE code #6354
Conversation
2254fa1
to
762f55d
Compare
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.
LGTM, a few small suggestions you can squash immediately
762f55d
to
e17d729
Compare
e17d729
to
f2de5fd
Compare
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.
You can squash immediately.
#include <string> | ||
#include <vector> | ||
|
||
#include "Evolution/Systems/Cce/Tags.hpp" |
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 CI clang-tidy is related to this include. I believe you don't actually need this in the hpp and can move it to the cpp, which should fix the issue.
And rename the file to just WorldtubeModeRecorder
f2de5fd
to
38f45aa
Compare
Proposed changes
This is sort of just a mish-mash of various changes I've made while updating the
ReduceCceWorldtube
executable. Most of it is just clarifying/organizing code that's shared. Future PRs will make more substantial changes toReduceCceWorldtube
.Towards #6246
Upgrade instructions
Code review checklist
make doc
to generate the documentation locally intoBUILD_DIR/docs/html
.Then open
index.html
.code review guide.
bugfix
ornew feature
if appropriate.Further comments