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
Added wrapper over some more parts of API #2
Conversation
@barnwal-paras lets work on one PR for now. Keep your work on one branch, unless if you have to experiment on something, and keep only one PR open. Please update the branches as needed, and close one of the two PRs. Thanks |
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.
@barnwal-paras nice that you separated the unit tests from the actual implementation.
following some comments for the next steps:
- Please add to gitignore the binaries and objects
- Make Examples part of the solution, so if one open Restcomm.sln should have Examples also
- Change the solution name to Restcomm-Connect-DotNet-SDK or something similar that will be clear this is the dotnet sdk
- Make the solution a nuget so a developer can include it in his project
- Change the namespace to something that will be clear this is the dotnet sdk, such as
org.restcomm.connect.sdk.dotnet
- Use the json endpoints
- Base URL must be passed as an argument to the main constructor of the SDK and NOT be hardcoded
The most important of all is the Unit Test should work with a mock of Restcomm-Connect and NOT using the cloud.restcomm.com. Ping me if you need help on that but its very important that you don't test against cloud.restcomm.com
I guess you are working on the following:
- Tutorial
- Documentation
src/AccountManager.cs
Outdated
public accountProperties Properties; | ||
|
||
|
||
public static string baseurl="https://cloud.restcomm.com/restcomm/2012-04-24/"; |
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.
@barnwal-paras base url should be configurable. The SDK will be used either with cloud.restcomm.com or any other Restcomm-Connect installation
src/AccountManager.cs
Outdated
|
||
|
||
|
||
namespace RestComm |
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.
@barnwal-paras what is the convention for namespace? Can we use org.restcomm.connect.sdk.dotnet
?
src/AccountManager.cs
Outdated
{ | ||
|
||
|
||
RestClient client = new RestClient(baseurl+"Accounts/"+sid); |
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.
@barnwal-paras we should use the JSON endpoints all over the project
gitignore added and testing is done on mock server plus using json instead of xml endpoints.
@barnwal-paras there must be a problem with the solution setup. When I open Restcomm.sln the classes in the src folder doesn't exist. Am I missing something? Can you please check? Also please open issues for the missing items/tasks. Like this we merge this initial PR and your work from now on will be with PRs based on issues. |
Also provide README.md with the following:
Also please provide example application and instructions for how to run it etc. Imagine you have a dotnet developer but not familiar with Restcomm-Connect, that wants to use the library. Given that, we must provide any necessary information for him to get started. |
I am working on documentation and description . Will update that on github by tomorrow . |
My bad . Actually i forgot to delete duplicate project file restcomm.csproj .please open solution file (Restcomm-dot-net-sdk.sln) or open Restcomm-dot-net-sdk.csproj inside Restcomm-dot-net-sdk folder to load the solution . |
Ok great, checking now @barnwal-paras |
Great work @barnwal-paras 👍 |
No description provided.