Skip to content

GEODE-3421: Cleanup public API headers#145

Merged
dgkimura merged 4 commits into
apache:developfrom
dgkimura:feature/GEODE-3421
Nov 10, 2017
Merged

GEODE-3421: Cleanup public API headers#145
dgkimura merged 4 commits into
apache:developfrom
dgkimura:feature/GEODE-3421

Conversation

@dgkimura

@dgkimura dgkimura commented Nov 7, 2017

Copy link
Copy Markdown
Member

Move the following out of the public API:

  • InternalCacheTransactionManager2PC
  • Log.hpp
  • utils.hpp

Testing:

  • Built on Mac/Windows
  • Unit/integration tests passed on Mac/Windows

#include <geode/utils.hpp>
#include <geode/CacheableString.hpp>

#include "util/functional.hpp"

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

While I like the rename, I think this one header should probably remain public. It provides the hash and equal_to for dereferenced shared_ptrobjects. It could be quite helpful. Though I suppose we can move it public later if people are asking for it.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unless you have strong opinion, I think I am going to stick with your second approach to keep it private until people are asking for it.

@jake-at-work

Copy link
Copy Markdown
Contributor

What about Assert.hpp? Seems like it shouldn't be in the public API?

@dgkimura

dgkimura commented Nov 9, 2017

Copy link
Copy Markdown
Member Author

Ah, I think you're right about Assert.hpp. I hadn't considered since it wasn't in the ticket.

@jake-at-work jake-at-work left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good

@dgkimura dgkimura merged commit 2acb708 into apache:develop Nov 10, 2017
mmartell pushed a commit to mmartell/geode-native that referenced this pull request Mar 6, 2018
GEODE-3421: Cleanup public API headers
mmartell pushed a commit to mmartell/geode-native that referenced this pull request Mar 6, 2018
GEODE-3421: Cleanup public API headers
jake-at-work pushed a commit to jake-at-work/geode-native that referenced this pull request Oct 6, 2018
GEODE-3421: Cleanup public API headers
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants