Skip to content

SDK: Add get_property support, remove callbacks and vectors from properties#153

Merged
JeffreySaathoff merged 5 commits intodevelopfrom
user/jeffreys/getProperty
Feb 8, 2023
Merged

SDK: Add get_property support, remove callbacks and vectors from properties#153
JeffreySaathoff merged 5 commits intodevelopfrom
user/jeffreys/getProperty

Conversation

@JeffreySaathoff
Copy link
Contributor

No description provided.

@JeffreySaathoff JeffreySaathoff requested a review from a team as a code owner February 2, 2023 23:51
@JeffreySaathoff
Copy link
Contributor Author

JeffreySaathoff commented Feb 3, 2023

@microsoft-github-policy-service agree company="Microsoft"


In reply to: 1414553893


In reply to: 1414553893

~download_property_value() = default;

static std::error_code make(const std::string& val, download_property_value& out);
static std::error_code make(const char* val, download_property_value& out) { return make(std::string(val), out); }
Copy link
Collaborator

@shishirb-MSFT shishirb-MSFT Feb 3, 2023

Choose a reason for hiding this comment

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

const char*

Without the const char* overload, doesn't the compiler automatically call the const std::string& version? #Closed

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Haven't looked at the code generation, but it picks the wrong overload (probably bool), so we need these overloads.

std::error_code as(std::vector<unsigned char>& val) const noexcept;
std::error_code as(status_callback_t& val) const noexcept;

#if defined(DO_INTERFACE_COM)
Copy link
Collaborator

@shishirb-MSFT shishirb-MSFT Feb 3, 2023

Choose a reason for hiding this comment

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

I'm convinced about this, so we can remove the ifdef. #Closed

This is so any user of the SDK does not have to worry about supplying platform specific compile definitions to use the SDK
*/
// Values for download_property::security_flags
enum class download_security_flags : uint32_t
Copy link
Collaborator

@shishirb-MSFT shishirb-MSFT Feb 3, 2023

Choose a reason for hiding this comment

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

download_security_flags

Want to talk about this tomorrow. #Closed

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Discussed offline. Removing for now.

explicit unique_variant(const VARIANT& other) noexcept; // takes ownership via shallow copy
unique_variant(const unique_variant& other); // true copy
unique_variant(unique_variant&& other) noexcept;
unique_variant& operator=(unique_variant&& other);
Copy link
Collaborator

@shishirb-MSFT shishirb-MSFT Feb 3, 2023

Choose a reason for hiding this comment

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

operator=

Move assignment also must be noexcept. #Closed

using native_type = boost::variant<std::string, uint32_t, uint64_t, bool, std::vector<unsigned char>>;
using native_type = boost::variant<std::string, uint32_t, uint64_t, bool>;
#endif
CDownloadPropertyValueInternal();
Copy link
Collaborator

@shishirb-MSFT shishirb-MSFT Feb 3, 2023

Choose a reason for hiding this comment

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

CDownloadPropertyValueInternal

We probably don't need any of the user-defined c'tor/d'tor now. #Closed

@shishirb-MSFT
Copy link
Collaborator

shishirb-MSFT commented Feb 8, 2023

FYI - when completing the PR, github adds all commit messages into the squashed commit. Need to remove/re-format as needed before clicking the 'confirm merge' button.
Also, add the "SDK:" prefix to the title before merging.. helps in building the release notes in future.

Copy link
Collaborator

@shishirb-MSFT shishirb-MSFT left a comment

Choose a reason for hiding this comment

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

:shipit:

@JeffreySaathoff JeffreySaathoff changed the title Add get_property support, remove callbacks and vectors from properties SDK: Add get_property support, remove callbacks and vectors from properties Feb 8, 2023
@JeffreySaathoff JeffreySaathoff merged commit 17b1997 into develop Feb 8, 2023
@JeffreySaathoff JeffreySaathoff deleted the user/jeffreys/getProperty branch February 8, 2023 20:27
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