Skip to content
This repository has been archived by the owner on Mar 20, 2024. It is now read-only.

Replace voice name with voice style in NCCO #44

Closed
wants to merge 2 commits into from
Closed

Replace voice name with voice style in NCCO #44

wants to merge 2 commits into from

Conversation

thearavind
Copy link

@thearavind thearavind commented Oct 3, 2020

Fixes #41

Checklist:

  • I have read the contributor's guide.
  • I linked an issue in the previous section
  • I was assigned the linked issue
  • I have tested the change to the best of my ability
  • My change requires a changes to the documentation.

In this PR I have added support for voice styles and language fields without removing the existing voice name field. They both will co-exist

Copy link
Contributor

@lornajane lornajane left a comment

Choose a reason for hiding this comment

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

Thanks! There are a few more things that are needed here but I'm happy to see this change happening.

Could you add a description please? We don't have detailed pull request templates on this repo but some idea of what's changing would be good.

There are some test failures, and I'd recommend that you also rebase onto newest master to avoid the conflict resulting from an unrelated change.

I can't tell if you edited the files in internal or regenerated them but we need to keep the voice_name API field as well - I realise that's not what is in the OpenAPI description at the moment but there's a discussion about this here https://github.com/Nexmo/api-specification/pull/367. Could you reinstate the Voice Name as well as adding the new style field? Also check if the language field is present everywhere the style field is used (they are a pair).

@thearavind
Copy link
Author

Hi @lornajane I will try to add some unit test cases in the next commit, Also I'll regenerate the code into the internal folder using the updated OpenAPI spec files after https://github.com/Nexmo/api-specification/pull/367 gets merged

@lornajane
Copy link
Contributor

That PR is causing some internal discussion so I have added the hacktoberfest-accepted label to this PR to make sure you get the credit for your work! I'll merge or update this once the change to the API description has been made.

@thearavind thearavind closed this by deleting the head repository Oct 13, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Move from Voice Names to Voice Styles
2 participants