-
Notifications
You must be signed in to change notification settings - Fork 592
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
Merge Properties and PropertiesI, switch to string_view #1914
Conversation
@@ -13,11 +13,14 @@ | |||
#include "Initialize.h" | |||
#include "PluginF.h" | |||
#include "Properties.h" | |||
#include "ProxyF.h" | |||
#include "Proxy.h" |
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.
My cleanup of the headers included by Properties.h revealed a bug in this include list.
@@ -117,7 +117,7 @@ ICE_API PropertiesPtr createProperties(); | |||
* @return A new property set initialized with the property settings | |||
* that were removed from the argument vector. | |||
*/ | |||
ICE_API PropertiesPtr createProperties(StringSeq& seq, const PropertiesPtr& defaults = 0); | |||
ICE_API PropertiesPtr createProperties(StringSeq& seq, const PropertiesPtr& defaults = nullptr); |
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 am wonder if we should just remove these functions that are now equivalent to a call to make_shared<Properties>(...)
_properties = p._properties; | ||
} | ||
|
||
Ice::Properties::Properties(StringSeq& args, const PropertiesPtr& defaults) |
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.
It's just the implementation that was in PropertiesI - I didn't change anything.
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.
Looks good!
This PR merges Properties and PropertiesI and replaces "const string&" by string_view in the Properties API.