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

DEVEXP-269 (Part 1/2): Refactor Voice models #24

Merged
merged 5 commits into from
Feb 22, 2024

Conversation

asein-sinch
Copy link
Collaborator

This PR is mainly about some refactoring among generated models. Nothing major here.

Only notable change: voice.applications.getNumbers becomes voice.applications.listNumbers to stay consistent with our naming => To be updated in other SDKs

@asein-sinch asein-sinch requested a review from a team February 21, 2024 14:34
Copy link

@JPPortier JPPortier left a comment

Choose a reason for hiding this comment

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

Not reviewed all generated files, but it could be interesting to identify from oas generator tool repo itself what cause these refactoring onto existing sources
So: we won't have to review generated sources but the generator itself and it enhancements

import { VoicePrice } from '../voice-price';
import { Participant } from '../participant';

export interface CallObject {

Choose a reason for hiding this comment

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

It took me some time to identify what mean "CallObject" and it usage.
Finally have to ensure I was right by looking at the OAS file details to identify it is the getCallResponseObj schema.
So finally, may be it could mean the name should convey more information for auto-documentation purpose and readability for maintainers

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Naming is a complicated task :)
The main object here is a 'Call' which is also the name of the service 'Calls' and also the verb 'call'. This is why I've added the suffix 'Object' to be clear about which kind of call it was and to avoid a naming conflict.
Then, the operationId is Calling_GetCallResult but here "Result" doesn't mean "Response" but rather "Status" which confused me too. The response object defined in the specification is getCallResponseObj which is "the object that represents the response of a getCall (status) query". So I thought to name it "CallResponse" but then it would be confusing with the actions of the "Call" service.
Long story short: what do you think about CallInformation?

Choose a reason for hiding this comment

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

Yes; and it is not so far from documentation related to operation: Get information about a call

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Fixed. In the end it's GetCallInformation as it is the result of a GET call, such as "get callbacks" for instance

…tructions (#25)

* DEVEXP-269: Add helpers for building SVAML actions and instructions

* Add 'call' function for conference API

* Add AMD 

* Add transcription locale
@asein-sinch asein-sinch merged commit 16c95c0 into main Feb 22, 2024
2 checks passed
@asein-sinch asein-sinch deleted the DEVEXP-269_Voice-helpers branch February 22, 2024 14:00
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.

2 participants