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

[Merged by Bors] - feat: added vfnlu types to base-types [LUIS Removal] (PL-532) #443

Closed
wants to merge 4 commits into from
Closed

[Merged by Bors] - feat: added vfnlu types to base-types [LUIS Removal] (PL-532) #443

wants to merge 4 commits into from

Conversation

zhihil
Copy link
Contributor

@zhihil zhihil commented Jun 15, 2023

Fixes or implements PL-532

Brief description. What is this change?

Implementation details. How do you make this change?

Setup information

Deployment Notes

Related PRs

Checklist

  • Breaking changes have been communicated, including:
    • New required environment variables
    • Renaming of interfaces (API routes, request/response interface, etc)
  • New environment variables have been deployed
  • Appropriate tests have been written
    • Bug fixes are accompanied by an updated or new test
    • New features are accompanied by a new test

@zhihil zhihil self-assigned this Jun 15, 2023
zhihil
zhihil commented Jun 15, 2023
View reviewed changes
Copy link
Contributor Author

@zhihil zhihil left a comment

Choose a reason for hiding this comment

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

Some comments

packages/voiceflow-types/src/luis/types.ts
@@ -1,5 +1,10 @@
import { SlotType } from '../constants';
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Most of the types can be left alone because they are only used in LUIS model import/export, which is something we will continue to support. However,BuiltInLuisSlotType and GENERAL_SLOT_TYPE_TO_LUIS are used in the NLU training process and we've marked these as deprecated to discourage further usage.

packages/voiceflow-types/src/vfnlu/types.ts
@@ -0,0 +1,76 @@
import { SlotType } from '../constants';
Copy link
Contributor Author

@zhihil zhihil Jun 15, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

This file contains BuiltInVFNLUSlotType and GENERAL_SLOT_TYPE_TO_VFNLU, which are just copy-pastes of BuiltInLuisSlotType and GENERAL_SLOT_TYPE_TO_LUIS. This is subject to change as the ML team evolves VFNLU.

In the future, we may want to remove/add slot types depending on what ML team wants to do with VFNLU.

@zhihil zhihil added the in progress In progress, should not be merged label Jul 7, 2023
@zhihil zhihil changed the title feat: added vfnlu types to base-types (PL-532) feat: added vfnlu types to base-types [VFNLU-CUT] (PL-532) Jul 7, 2023
@zhihil zhihil changed the title feat: added vfnlu types to base-types [VFNLU-CUT] (PL-532) feat: added vfnlu types to base-types (PL-532) Jul 7, 2023
@zhihil zhihil modified the milestone: VFNLU Cutover Jul 7, 2023
@zhihil zhihil changed the title feat: added vfnlu types to base-types (PL-532) feat: added vfnlu types to base-types [VFNLU Cutover] (PL-532) Jul 7, 2023
@sonarcloud
Copy link

sonarcloud bot commented Jul 7, 2023

SonarCloud Quality Gate failed.    Quality Gate failed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell C 32 Code Smells

No Coverage information No Coverage information
0.0% 0.0% Duplication

idea Catch issues before they fail your Quality Gate with our IDE extension sonarlint SonarLint

@zhihil zhihil changed the title feat: added vfnlu types to base-types [VFNLU Cutover] (PL-532) feat: added vfnlu types to base-types [LUIS Removal] (PL-532) Sep 11, 2023
@zhihil zhihil added ready to merge and removed in progress In progress, should not be merged labels Sep 19, 2023
@zhihil
Copy link
Contributor Author

zhihil commented Sep 19, 2023

bors r+

@zhihil
Copy link
Contributor Author

zhihil commented Sep 19, 2023

bors cancel

@bors-vf
Copy link

bors-vf bot commented Sep 19, 2023

Canceled.

bors-vf bot pushed a commit that referenced this pull request Sep 19, 2023
@zhihil
feat: added vfnlu types to base-types [LUIS Removal] (PL-532) (#443)
24fef50 
<!-- You can erase any parts of this template not applicable to your Pull Request. -->

**Fixes or implements PL-532**

### Brief description. What is this change?

<!-- Build up some context for your teammates on the changes made here and potential tradeoffs made and/or highlight any topics for discussion -->
@sonarcloud
Copy link

sonarcloud bot commented Sep 19, 2023

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

77.8% 77.8% Coverage
0.0% 0.0% Duplication

warning The version of Java (11.0.3) you have used to run this analysis is deprecated and we will stop accepting it soon. Please update to at least Java 17.
Read more here

@zhihil
Copy link
Contributor Author

zhihil commented Sep 19, 2023

bors r+

bors-vf bot pushed a commit that referenced this pull request Sep 19, 2023
@zhihil
feat: added vfnlu types to base-types [LUIS Removal] (PL-532) (#443)
35967fb 
<!-- You can erase any parts of this template not applicable to your Pull Request. -->

**Fixes or implements PL-532**

### Brief description. What is this change?

<!-- Build up some context for your teammates on the changes made here and potential tradeoffs made and/or highlight any topics for discussion -->
@zhihil
Copy link
Contributor Author

zhihil commented Sep 19, 2023

bors cancel

@bors-vf
Copy link

bors-vf bot commented Sep 19, 2023

Canceled.

@zhihil
Copy link
Contributor Author

zhihil commented Sep 19, 2023

bors r+

bors-vf bot pushed a commit that referenced this pull request Sep 19, 2023
@zhihil
feat: added vfnlu types to base-types [LUIS Removal] (PL-532) (#443)
416646a 
<!-- You can erase any parts of this template not applicable to your Pull Request. -->

**Fixes or implements PL-532**

### Brief description. What is this change?

<!-- Build up some context for your teammates on the changes made here and potential tradeoffs made and/or highlight any topics for discussion -->
@bors-vf
Copy link

bors-vf bot commented Sep 19, 2023

Pull request successfully merged into master.

Build succeeded:

@bors-vf bors-vf bot changed the title feat: added vfnlu types to base-types [LUIS Removal] (PL-532) [Merged by Bors] - feat: added vfnlu types to base-types [LUIS Removal] (PL-532) Sep 19, 2023
@bors-vf bors-vf bot closed this Sep 19, 2023
@bors-vf bors-vf bot deleted the brennan/remove-luis/PL-532 branch September 19, 2023 15:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@DecathectZero DecathectZero DecathectZero approved these changes

Assignees

@zhihil zhihil

Labels
ready to merge
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@zhihil @DecathectZero