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

Updating cognitiveservices-luis-authoring SDK #4792

Merged

Conversation

ahmedabuamra
Copy link
Contributor

@ahmedabuamra ahmedabuamra commented Aug 18, 2019

All lines of code in this PR are auto-generated except updating the README.md file.

Copy link
Contributor

@ramya-rao-a ramya-rao-a left a comment

Choose a reason for hiding this comment

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

@ahmedabuamra Some of the comments I made in #4791 applies here as well, so please take a look at those.

Can you update the PR description with list of changes that you had to do manually? Please do the same for the other PR as well

@ahmedabuamra ahmedabuamra changed the title Updating cognitive luis authoring sdk Updating cognitiveservices-luis-authoring SDK Aug 21, 2019
@ahmedabuamra
Copy link
Contributor Author

What I have changed:

  • Added the copyrights manually.
  • Replaced subscriptionId with authoringKey as @ChrisHMSFT suggested.
  • Changed the Impression in README manually.
  • Replaced the comment on versionId with @ramya-rao-a 's.

Copy link
Contributor

@ramya-rao-a ramya-rao-a left a comment

Choose a reason for hiding this comment

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

@kayousef mentioned that we want to auto publish this library. Therefore, please update the package version and enable the autopublish feature by adding back the "autoPublish": true in package.json file

@ahmedabuamra
Copy link
Contributor Author

ahmedabuamra commented Aug 23, 2019

@kayousef , @ramya-rao-a
I've updated the version and added "autoPublish": true in both Authoring and Runtime SDKs.

@ramya-rao-a
Copy link
Contributor

Since we have decided to get an update published with this PR approved, can we also get the browser sample updated?

let authoringKey = process.env["luis-authoring-key"];
const creds = new CognitiveServicesCredentials(authoringKey);
const region = "<your-region>";
const client = new LUISAuthoringClient(
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
const client = new LUISAuthoringClient(
const client = new Azure.CognitiveservicesLuisAuthoring.LUISAuthoringClient(

Copy link
Contributor

Choose a reason for hiding this comment

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

@ahmedabuamra This change is still pending.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

sorry I missed it.

Copy link
Contributor

@ramya-rao-a ramya-rao-a 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!

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.

4 participants