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

[communication] Fix SDK_VERSION value #12546

Merged

Conversation

DominikMe
Copy link
Member

We mistakingly used the service api version for constructing the SDK user agent string.
Following the pattern of other services and hard-coding the version string.

import { apiVersion } from "./generated/src/models/parameters";

export const SDK_VERSION: string = apiVersion.mapper.defaultValue;
export const SDK_VERSION: string = "1.0.0-beta.3";
Copy link
Member

Choose a reason for hiding this comment

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

What is the logic of the other repos to hardcode the SDK version?

This feels trappy where updating the SDK version requires changes in an unknown number of places.

Copy link
Member Author

Choose a reason for hiding this comment

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

It's not great but the other JS libraries hard-code too.

There are at least 3 changes to make with a version bump: Changelog, package-version for autorest, and SDK_VERSION here.

Copy link
Member

@juancamilor juancamilor left a comment

Choose a reason for hiding this comment

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

👍

@DominikMe
Copy link
Member Author

/azp run js - communication - ci

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@DominikMe DominikMe merged commit 1bfc363 into Azure:master Nov 14, 2020
@DominikMe DominikMe deleted the communication-fix-sdk-version-constant branch November 14, 2020 00:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants