-
Notifications
You must be signed in to change notification settings - Fork 8
Release r1.2 with click-to-dial v0.1.0-rc.1 #65
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
base: main
Are you sure you want to change the base?
Changes from all commits
7c34ed9
d696dff
0a4264f
24048f5
3794c52
9ce324a
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Same comments on the other 2 feature files, but with resource adapted to .../{callId} and .../{callId}/recording respectively
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Thanks @tanjadegroot — addressed in PR #66. We updated the Background sections of all three There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The different error codes are not covered by test scenarios which would be useful for API consumers. However, you can address this also in a later version. But would recommmend to set item 8 (Enhanced test cases) = in the API readiness checklist to "N" at this point.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Agreed — item 8 (Enhanced test cases) has been set to "N" in the API Readiness Checklist in commit 9ce324a. We'll add error-code-specific test scenarios in a future iteration. |
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I would recommend not to duplicate release information in the API documentation as it is captured in the CHANGELOG.md and the README.md and will be handled automatically in the future. Most API documentation should be in the API yaml file which should be self sufficient. Additional info like the curl commands can be here to be part the release. Other information is up to you.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. |
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.
I noticed the use of the "subject" property in the CloudEvent structure (line 558) which is not supported by CAMARA. as the same information is already in the "data" property of the event, it can simply be removed.
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 @tanjadegroot — fixed in PR #67 by removing subject from the CloudEvent schema/required list and the example. Any remaining documentation mentions of subject were updated in PR #68.