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

Make storage library static #663

Merged
merged 1 commit into from
May 11, 2021
Merged

Make storage library static #663

merged 1 commit into from
May 11, 2021

Conversation

LaurentiuCristofor
Copy link
Contributor

The storage library had remained declared as SHARED instead of STATIC, which leads to failures because it's not picked up by the gaia package.

Copy link
Contributor

@simone-gaia simone-gaia left a comment

Choose a reason for hiding this comment

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

LGTM, it would be ideal to have a test in test_sdk to ensure this won't happen again.

@LaurentiuCristofor
Copy link
Contributor Author

LaurentiuCristofor commented May 11, 2021

LGTM, it would be ideal to have a test in test_sdk to ensure this won't happen again.

I'm not sure how we can test for this. test_sdk runs within our general build framework, so that's why it continued to work. The problem is that we'd need to install the sdk and then test it in that context, but we don't have such a test as part of the local build - only as part of the TeamCity build, which is how this issue surfaced.

Also, this is a leftover from the time when all our libraries were SHARED. When we first built the SDK, we updated them to STATIC but this library was left unchanged because it wasn't used by the system yet. This kind of issue is unlikely to happen in the future and if it does, it will get caught by the TeamCity testing, which I think is good enough.

@LaurentiuCristofor LaurentiuCristofor merged commit f244e8f into master May 11, 2021
@LaurentiuCristofor LaurentiuCristofor deleted the laur_sdk_fix branch May 11, 2021 20:23
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.

5 participants