Skip to content

DEVEXP-320: Add support for Emails#34

Merged
asein-sinch merged 8 commits intoDEVEXP-316_Fax-API_Faxesfrom
DEVEXP-320_Fax-API_Emails
Mar 15, 2024
Merged

DEVEXP-320: Add support for Emails#34
asein-sinch merged 8 commits intoDEVEXP-316_Fax-API_Faxesfrom
DEVEXP-320_Fax-API_Emails

Conversation

@asein-sinch
Copy link
Copy Markdown
Collaborator

No description provided.

@asein-sinch asein-sinch requested a review from a team February 29, 2024 13:23
// ----------------------------------------------
// Method 1: Fetch the data page by page manually
// ----------------------------------------------
let response= await sinchClient.fax.emails.list(requestData);
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Suggested change
let response= await sinchClient.fax.emails.list(requestData);
let response = await sinchClient.fax.emails.list(requestData);

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Fixed

// ----------------------------------------------
// Method 1: Fetch the data page by page manually
// ----------------------------------------------
let response= await sinchClient.fax.emails.listNumbers(requestData);
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Suggested change
let response= await sinchClient.fax.emails.listNumbers(requestData);
let response = await sinchClient.fax.emails.listNumbers(requestData);

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Fixed

// ----------------------------------------------
// Method 1: Fetch the data page by page manually
// ----------------------------------------------
let response= await sinchClient.fax.services.listEmailsForNumber(requestData);
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Suggested change
let response= await sinchClient.fax.services.listEmailsForNumber(requestData);
let response = await sinchClient.fax.services.listEmailsForNumber(requestData);

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Fixed

const headers: { [key: string]: string | undefined } = {
'Content-Type': 'application/json',
'Accept': 'application/json',
};
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Those headers seem to be the same for all EmailsApi operation. Maybe it will be better to not duplicate the code and use class variable for that?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

I don't think it would be better to factorize in the code what can be (and actually is) defined individually at operation level in the specification. If the specification is updated later on and changes a single operation definition, the impacts on this generated code will be much bigger to apprehend than a direct regeneration.
What is bothering you exactly in this file? Is it the repetition of the same string? Is it the object definition itself?

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

yeah; just the repetition thing. Personally I like more concise source files.

Base automatically changed from DEVEXP-317_Fax-API_Callbacks to DEVEXP-316_Fax-API_Faxes March 5, 2024 14:57
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Looks like similar situation as in #32

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Removed. See PR#36

@650elx 650elx self-requested a review March 14, 2024 21:00
@asein-sinch asein-sinch merged commit 3238e8c into DEVEXP-316_Fax-API_Faxes Mar 15, 2024
@asein-sinch asein-sinch deleted the DEVEXP-320_Fax-API_Emails branch March 15, 2024 07:59
asein-sinch added a commit that referenced this pull request Mar 15, 2024
* DEVEXP-316: Add support for Faxes

* DEVEXP-317: Add support for Callbacks (#33)

* DEVEXP-320: Add support for Emails (#34)
asein-sinch added a commit that referenced this pull request Mar 15, 2024
* DEVEXP-316: Add support for Faxes (#32)

* DEVEXP-317: Add support for Callbacks (#33)

* DEVEXP-320: Add support for Emails (#34)
asein-sinch added a commit that referenced this pull request Mar 15, 2024
* DEVEXP-315: Add support for Services (#31)

* DEVEXP-316: Add support for Faxes (#32)

* DEVEXP-317: Add support for Callbacks (#33)

* DEVEXP-320: Add support for Emails (#34)
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