-
Notifications
You must be signed in to change notification settings - Fork 1
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
Feat/fax api faxes #48
Conversation
Co-authored-by: Volodymyr Lisovskyi <erehonvl@gmail.com>
Co-authored-by: Volodymyr Lisovskyi <erehonvl@gmail.com>
Co-authored-by: Volodymyr Lisovskyi <erehonvl@gmail.com>
Co-authored-by: Volodymyr Lisovskyi <erehonvl@gmail.com>
…ime` property in the `Fax.cs` file, the addition of a new `Download` method in the `Faxes.cs` file, and the update of package versions in the `Sinch.csproj` file. 1. The `CreateTime` property in the `Fax.cs` file under the `Sinch.Faxes` namespace has been altered. Previously, it was a string type, but it has now been changed to a DateTime type. This change will ensure that the `CreateTime` property holds date and time information in a more structured and accurate manner. 2. A new asynchronous method named `Download` has been added to the `Faxes.cs` file under the `Sinch.Faxes` namespace. This method accepts an id as a parameter and returns a Stream. This addition will allow users to download fax data asynchronously using the provided id. 3. The versions of the `Microsoft.Extensions.Http` and `System.Text.Json` packages in the `Sinch.csproj` file have been updated. The `Microsoft.Extensions.Http` package has been updated from version 7.0.0 to 8.0.0, and the `System.Text.Json` package has been updated from version 7.0.3 to 8.0.2. These updates will ensure that the project uses the latest and most secure versions of these packages. References: - `Fax.cs` file under the `Sinch.Faxes` namespace for the `CreateTime` property change. - `Faxes.cs` file under the `Sinch.Faxes` namespace for the new `Download` method. - `Sinch.csproj` file for the package version updates.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice work! I've put some comments that also apply to the implementation of this API in the Node.js SDK 🙂
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the review Antoine! Here you will find adjustment to mocks https://github.com/sinch/doppelganger/pull/25
Added an override for send fax https://github.com/sinch/doppelganger/pull/26 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Well done. Regarding the PR organization, maybe you can create another branch from main where all the sub-features for Fax (this PR being one of the sub-features) will be merged. Otherwise, I fear it will be a mess if you need to come back to the Faxes part once Email will be merged
Introduce Fax Api support
This PR brings in FaxClient interface and implementation of Faxes tag.
This is a PR sink for other fax related PRs as well.
Will be merged only when other child PRs regarding other tags will be merge into current.