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.

Sign up for GitHub

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

Jump to bottom

language: put sdk versions into package.json #4122

Merged
5 commits merged into from
Jan 21, 2020
Merged

language: put sdk versions into package.json #4122

5 commits merged into from
Jan 21, 2020

Conversation

ghost
Copy link

@ghost ghost commented Jan 20, 2020

The typescript library versions of our support libraries are now given
by the sdk version.

CHANGELOG_BEGIN
CHANGELOG_END

Pull Request Checklist

  • Read and understand the contribution guidelines
  • Include appropriate tests
  • Set a descriptive title and thorough description
  • Add a reference to the issue this PR will solve, if appropriate
  • Include changelog additions in one or more commit message bodies between the CHANGELOG_BEGIN and CHANGELOG_END tags
  • Normal production system change, include purpose of change in description

NOTE: CI is not automatically run on non-members pull-requests for security
reasons. The reviewer will have to comment with /AzurePipelines run to
trigger the build.

@ghost ghost force-pushed the sdk_versions_in_package_json branch from 937b756 to c4039df Compare January 20, 2020 17:18
aherrmann-da
aherrmann-da approved these changes Jan 20, 2020
View reviewed changes
Copy link
Contributor

@aherrmann-da aherrmann-da left a comment

Choose a reason for hiding this comment

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

Looks great, thank you!

The local attribute is not needed.

bazel_tools/sdk_version.bzl Outdated Show resolved Hide resolved
hurryabit
hurryabit reviewed Jan 20, 2020
View reviewed changes
Copy link
Contributor

@hurryabit hurryabit left a comment

Choose a reason for hiding this comment

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

Very nice. Thank you so much.

I'll leave the technical approval to @aherrmann.

hurryabit
hurryabit suggested changes Jan 21, 2020
View reviewed changes
Copy link
Contributor

@hurryabit hurryabit left a comment

Choose a reason for hiding this comment

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

I'm wondering if we should put 0.0.0 instead of SDK_VERSION into the package.json files. This would make the local development experience nicer. Of course, there's a very small risk that we ever need to use another package at version 0.0.0. However, I would resolve that issue when it occurs, if it ever does.

@aherrmann-da
Copy link
Contributor

I'm wondering if we should put 0.0.0 instead of SDK_VERSION into the package.json files. This would make the local development experience nicer. Of course, there's a very small risk that we ever need to use another package at version 0.0.0. However, I would resolve that issue when it occurs, if it ever does.

In rules_nodejs they use 0.0.0-PLACEHOLDER for that purpose. We could use a similar suffix to avoid collision. Such suffixes are allowed in semver.

@hurryabit
Copy link
Contributor

I like @aherrmann's idea. Let's do that.

@ghost ghost force-pushed the sdk_versions_in_package_json branch 2 times, most recently from 369707d to d8ab3b3 Compare January 21, 2020 12:44
@ghost
Copy link
Author

ghost commented Jan 21, 2020 •
edited by ghost
Loading

ok, we're using 0.0.0-SDK_VERSION now in the package.json.

@ghost ghost force-pushed the sdk_versions_in_package_json branch from d8ab3b3 to 043ebe7 Compare January 21, 2020 12:57
Robin Krom added 4 commits January 21, 2020 15:00
language: put sdk versions into package.json
03df3a2 
The typescript library versions of our support libraries are now given
by the sdk version.

CHANGELOG_BEGIN
CHANGELOG_END
removed local field
e4bffdd
better placeholders
040bc6d
consistent SDK_VERSION
d7657d3
@ghost ghost force-pushed the sdk_versions_in_package_json branch from 8465ca6 to b7554c0 Compare January 21, 2020 14:00
@ghost ghost force-pushed the sdk_versions_in_package_json branch from b7554c0 to 01b71a6 Compare January 21, 2020 14:13
hurryabit
hurryabit approved these changes Jan 21, 2020
View reviewed changes
Copy link
Contributor

@hurryabit hurryabit left a comment

Choose a reason for hiding this comment

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

Awesome. Thank you very much.

@ghost ghost merged commit ad93252 into master Jan 21, 2020
@ghost ghost deleted the sdk_versions_in_package_json branch January 21, 2020 14:41
This pull request was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@aherrmann-da @hurryabit