Skip to content

Conversation

@JeffreySaathoff
Copy link
Contributor

No description provided.

@JeffreySaathoff JeffreySaathoff requested a review from a team as a code owner February 14, 2023 01:55
return set_property(download_property::cost_policy, static_cast<uint32_t>(value));
}

// Returns existing downloads that are associated with the caller
Copy link
Collaborator

@shishirb-MSFT shishirb-MSFT Feb 15, 2023

Choose a reason for hiding this comment

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

that are associated with the caller

Not sure how to implement this in xplat since there is no 'caller identity'. I will leave it as not implemented until we figure it out. #Closed

{

enum class errc : int32_t
namespace errc
Copy link
Collaborator

@shishirb-MSFT shishirb-MSFT Feb 15, 2023

Choose a reason for hiding this comment

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

Why change to namespace? I think one of the std::error_code constructors expects enum (maybe we aren't using that c'tor today). #Closed

Copy link
Collaborator

@shishirb-MSFT shishirb-MSFT Feb 15, 2023

Choose a reason for hiding this comment

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

Ok I see the other changes in do_error_helpers.h and other files now.
Ignore my comment above.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I like it better without the typecasts

do_e_invalid_state = static_cast<int32_t>(0x80D02013), // DO_E_INVALID_STATE
};

constexpr auto e_not_impl = static_cast<int32_t>(0x80004001); // E_NOTIMPL
Copy link
Collaborator

@shishirb-MSFT shishirb-MSFT Feb 15, 2023

Choose a reason for hiding this comment

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

e_

I vote for removing the 'e_' and 'do_e_' prefixes since they are already inside an errc namespace. #Closed

Copy link
Collaborator

Choose a reason for hiding this comment

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

(prefixes were not required inside the enum either)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Agreed

ComPtr<IDOManager> manager;
RETURN_IF_FAILED(CoCreateInstance(__uuidof(DeliveryOptimization), nullptr, CLSCTX_LOCAL_SERVER, IID_PPV_ARGS(&manager)));
ComPtr<IEnumUnknown> spEnum;
RETURN_IF_FAILED(manager->EnumDownloads(pCategory, &spEnum));
Copy link
Collaborator

@shishirb-MSFT shishirb-MSFT Feb 15, 2023

Choose a reason for hiding this comment

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

EnumDownloads

CoSetProxyBlanket not required on manager because we need only identify level, which is the default, yes? #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.

Correct

download2->get_property(msdo::download_property::id, downloadId2);
ASSERT_EQ(downloadId1, downloadId2);

download2->start_and_wait_until_completion();
Copy link
Collaborator

@shishirb-MSFT shishirb-MSFT Feb 15, 2023

Choose a reason for hiding this comment

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

start_and_wait_until_completion

Probably should add a 1s wait after completion since job removal is async. #Closed

shishirb-MSFT
shishirb-MSFT previously approved these changes Feb 15, 2023
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:

@shishirb-MSFT shishirb-MSFT dismissed their stale review February 15, 2023 17:50

revoking review

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 merged commit 8df51fc into develop Feb 15, 2023
@JeffreySaathoff JeffreySaathoff deleted the user/jeffreys/enumDownloads branch February 15, 2023 18:35
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