DEVEXP-556: Add snippets for Numbers API #2
Conversation
asein-sinch
requested review from
JPPortier,
krogers0607,
Dovchik and
650elx
as code owners
|
I think makes sense to pull @alex-sberna into the reviewers |
snippets/numbers/list.js
Outdated
| type: 'LOCAL', | ||
| }; | ||
|
|
||
| // Method 1: Fetch the data page by page manually |
There was a problem hiding this comment.
I really like providing both methods, but I wonder if it would be better to have two snippets, one that lists page by page manually, and one that uses the iterator, rather than having both in the same file.
There was a problem hiding this comment.
Very good suggestion! There are now 2 snippets: list and list-auto
|
@asein-sinch WDYT about writing some tests for those snippets? Would be nice to have a certainty that they are actually working. |
@650elx , I have created tickets for ours SDKs: |
There was a problem hiding this comment.
outside of snippets content review, having different file names for each snippet will cause end users to perform manual renaming when it will be time to use them from quickstart repo (part of initial aim for these repos)
i.e.: ideally user should be able to copy/paste a generic snippet.jssources coming from this repo onto https://github.com/sinch/sinch-sdk-node-quickstart/pull/2/files#diff-7a3942f60eabc386bff6328326a40a3d6d7350f10da7252544de50e633693566 directory.
So it will overwrite the snippets.js file onto quickstart repo and just run npm start
A directory structure based like below could reduce work (and typing issue) for end user :
- numbers
- available-regions
- snippets.js
- callback-configuration
- get
- snippets.js
- update
- snippets.js
- get
- available-regions
...
No description provided.