Skip to content

chore: Remove instances of XXX_SDK_VERSION_XXX by reading version from package.json #952

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

Merged
merged 4 commits into from
Jul 17, 2020

Conversation

MathBunny
Copy link
Contributor

@MathBunny MathBunny commented Jul 16, 2020

Summary

Removes instances of XXX_SDK_VERSION_XXX which were substituted by gulp at the build stage, and instead reads from package.json the value of version.

Test Plan

Added unit test to verify that the value is read from package.json.

Edit: I had a thought of perhaps adding an integration test to it or for the example.test.ts, however this is a util function which isn't supposed to be exposed to developers hence my initial reaction was against that.

@MathBunny MathBunny changed the title Remove instances of XXX_SDK_VERSION_XXX by reading version from package.json chore: Remove instances of XXX_SDK_VERSION_XXX by reading version from package.json Jul 16, 2020
Copy link
Contributor

@hiranya911 hiranya911 left a comment

Choose a reason for hiding this comment

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

Idea is solid. Just need a couple of adjustments to how it's implemented and tested.

@hiranya911 hiranya911 assigned MathBunny and unassigned hiranya911 Jul 16, 2020
@MathBunny MathBunny requested a review from hiranya911 July 16, 2020 18:45
@MathBunny MathBunny assigned hiranya911 and unassigned MathBunny Jul 16, 2020
Copy link
Contributor

@hiranya911 hiranya911 left a comment

Choose a reason for hiding this comment

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

Thanks! LGTM 👍

@hiranya911 hiranya911 removed their assignment Jul 16, 2020
Copy link
Member

@lahirumaramba lahirumaramba left a comment

Choose a reason for hiding this comment

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

Thank you! LGTM!

@MathBunny MathBunny merged commit 92b0d37 into master Jul 17, 2020
@MathBunny MathBunny deleted the hlazu-sdk-version-number branch August 4, 2020 14:19
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.

3 participants