Page MenuHomePhabricator

[CommCoreModule] Fill out `sessionSignature(deviceID) => toSign` boilerplate in `Client`
ClosedPublic

Authored by atul on Feb 8 2022, 1:35 PM.
Tags
None
Referenced Files
F3301644: D3143.diff
Mon, Nov 18, 2:47 AM
Unknown Object (File)
Sat, Nov 2, 4:38 AM
Unknown Object (File)
Fri, Oct 25, 12:01 AM
Unknown Object (File)
Fri, Oct 25, 12:01 AM
Unknown Object (File)
Fri, Oct 25, 12:01 AM
Unknown Object (File)
Fri, Oct 25, 12:00 AM
Unknown Object (File)
Oct 7 2024, 9:14 PM
Unknown Object (File)
Oct 7 2024, 6:31 PM

Details

Summary

So we can hit SessionSignature(...) endpoint

Test Plan

Hit tunnelbroker with request and see if we get a response

Diff Detail

Repository
rCOMM Comm
Lint
No Lint Coverage
Unit
No Test Coverage

Event Timeline

Harbormaster returned this revision to the author for changes because remote builds failed.Feb 8 2022, 1:38 PM
Harbormaster failed remote builds in B6651: Diff 9416!
atul requested review of this revision.Feb 8 2022, 1:44 PM
This revision is now accepted and ready to land.Feb 8 2022, 1:47 PM
This revision now requires review to proceed.Feb 8 2022, 1:47 PM
ashoat added a reviewer: jim.
ashoat added inline comments.
native/cpp/CommonCpp/grpc/Client.cpp
138 ↗(On Diff #9416)

So what's going on here... NULL is a c-str and that get initialized into an std::string? Is this the best pattern?

This revision is now accepted and ready to land.Feb 8 2022, 9:40 PM
native/cpp/CommonCpp/grpc/Client.cpp
138 ↗(On Diff #9416)

NULL == nullptr since C++ 11 so the function would return nullptr

At the callsite we'll check the truthiness of the return value

138 ↗(On Diff #9416)

Can be more explicit and return nullptr?

native/cpp/CommonCpp/grpc/Client.cpp
138 ↗(On Diff #9416)

Just kidding... Looks like the right thing to do is

return std::string{};

and check for empty string

if (blah.empty()) {
  return;
}
native/cpp/CommonCpp/grpc/Client.cpp
138 ↗(On Diff #9416)

Alternatively I guess could return a std::optional<std::string>> or tuple of (grpc::Status, std::optional<std::string>) or something

replace NULL with empty string

This revision was landed with ongoing or failed builds.Feb 9 2022, 10:34 AM
This revision was automatically updated to reflect the committed changes.