Skip to content
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

RSDK-8343 - create public version_metadata file #290

Merged
merged 6 commits into from
Sep 23, 2024

Conversation

stuqdog
Copy link
Member

@stuqdog stuqdog commented Sep 20, 2024

No description provided.

@stuqdog stuqdog requested a review from a team as a code owner September 20, 2024 14:47
@stuqdog stuqdog requested review from purplenicole730 and removed request for a team September 20, 2024 14:47
@@ -115,6 +116,16 @@ BOOST_AUTO_TEST_CASE(test_from_dm_from_extra) {
BOOST_CHECK_EQUAL(from_dm_from_extra(map), false);
}

BOOST_AUTO_TEST_CASE(test_version_metadata) {
// we don't want to check the specific values because they're going to be changing,
Copy link
Contributor

Choose a reason for hiding this comment

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

How about we get the sdk_version() string here, and then check that it matches

std::to_string(sdk_major_version()) + "." + std::to_string(sdk_minor_version()) + "." + std::to_string(sdk_patch_version());

std::string sdk_version() {
std::string version_metadata(impl::k_version);
version_metadata.erase(0, version_metadata.find(';') + 1);
return version_metadata.substr(0, version_metadata.find(';'));
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe have the function body as

static std::string result = []{
    std::string version_metadata(impl::k_version);
    version_metadata.erase(0, version_metadata.find(';') + 1);
    return version_metadata.substr(0, version_metadata.find(';'));
}();

return result;

That way we only ever do this computation once.

This might seem like a micro optimization, but if this stuff later gets plugged into, eg, the response type of a grpc request that throws out the version, it could easily be hit over and over

Copy link
Member Author

Choose a reason for hiding this comment

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

ooh good thinking! thanks :)

}

int get_sub_version(int which) {
auto version = sdk_version();
Copy link
Contributor

Choose a reason for hiding this comment

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

Similarly here, we could do something like

static const std::array<int, 3> components =  [] {
    std::array<int, 3> result;
    
    // copy your function here but let the `for` loop parse the `int`s into the array

    return result;
}();

and then your function just returns components[i];

@stuqdog stuqdog merged commit f152998 into viamrobotics:main Sep 23, 2024
3 checks passed
@stuqdog stuqdog deleted the public-version-hpp branch September 23, 2024 14:41
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