Partial organization of mds/ header sections #11959
Conversation
|
|
||
| private: | ||
| void _notify_mdsmap(MDSMap const *mdsmap); | ||
| void _send(); |
There was a problem hiding this comment.
Did you move these first two private functions for a reason? It kind of looks like they go with C_MDS_BeaconSender to me?
...oh, because we're supposed to expose functions prior to members? Is that going to be feasible with our header-implemented functions?
There was a problem hiding this comment.
...oh, because we're supposed to expose functions prior to members?
That's what the style guide says but I'm not committed to that part of the style doc if we dislike it.
Is that going to be feasible with our header-implemented functions?
I don't see why not?
There was a problem hiding this comment.
The allowed order of declaration versus usage is annoying. I don't know the actual rules anymore and it seems to vary based on the compiler/settings.
| void operator delete(void *p) { | ||
| pool.free(p); | ||
| } | ||
| const Capability& operator=(const Capability& other); // no copying |
There was a problem hiding this comment.
Do the code style rules really require us to separate this and the copy constructor when disabling both? (Or does it perhaps only allow us to use better solutions?)
There was a problem hiding this comment.
Yes but I'm open to doing things differently. I personally think putting operators with the cons/des makes sense but the guide doesn't say anything about that.
|
Might as well run it through a test just in case though. |
|
jenkins test this please (eio ignored in master) |
This amends the code to follow our C++ style guidelines with the goal of increasing header readability. Signed-off-by: Patrick Donnelly <pdonnell@redhat.com>
This amends the code to follow our C++ style guidelines with the goal of increasing header readability. Signed-off-by: Patrick Donnelly <pdonnell@redhat.com>
This amends the code to follow our C++ style guidelines with the goal of increasing header readability. Signed-off-by: Patrick Donnelly <pdonnell@redhat.com>
batrick
force-pushed
the
mds-header-prot-org
branch
from
55b3171
to
7f96ace
Compare
|
@jcsp, any thoughts before we merge this? (Don't want it to get stale!) |
|
No complaints here! |
As we discussed in the CephFS summit, this is a simple organization of the class members in the mds/ headers following our C++ guidelines. The idea is to improve readability and understanding of each class' purpose.
I'll continue incrementally working on this as I find spare time/inclination.