Skip to content
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

fix(calling-docs): calling sdk api doc update #3106

Merged
merged 12 commits into from
Oct 3, 2023

Conversation

Kesari3008
Copy link
Contributor

@Kesari3008 Kesari3008 commented Sep 22, 2023

COMPLETES #https://jira-eng-gpk2.cisco.com/jira/browse/SPARK-428325

This pull request addresses

Calling SDK API doc changes

by making the following changes

Moved docs generation path from packages/calling/docs to docs/calling
Deleted docs folder from inside calling folder.
Updated the docs/index.html file to add the Calling API reference link which redirects to Calling SDK API docs.
Made changes in README.md file
Change in each class and interface files to have proper description of properties and methods.

Below modules have been changed in this PR:

CallHistory
CallSettings
Contacts

Change Type

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • Documentation update
  • Tooling change
  • Internal code refactor

The following scenarios where tested

< ENUMERATE TESTS PERFORMED, WHETHER MANUAL OR AUTOMATED >

I certified that

  • I have read and followed contributing guidelines

  • I discussed changes with code owners prior to submitting this pull request

  • I have not skipped any automated checks

  • All existing and new tests passed

  • I have updated the documentation accordingly


Make sure to have followed the contributing guidelines before submitting.

@Kesari3008 Kesari3008 requested a review from a team as a code owner September 22, 2023 20:53
@Kesari3008 Kesari3008 added the validated If the pull request is validated for automation. label Sep 22, 2023
@Kesari3008 Kesari3008 changed the title Spark 428325 calling sdk api doc update fix(calling-docs): Spark 428325 calling sdk api doc update Sep 22, 2023
@Kesari3008 Kesari3008 marked this pull request as draft September 25, 2023 04:48
@Kesari3008 Kesari3008 marked this pull request as ready for review September 26, 2023 09:15
Copy link
Contributor

@mkesavan13 mkesavan13 left a comment

Choose a reason for hiding this comment

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

Minor comments but a lot in number. To summarize,

  • Bring all param names and descriptions back. That is still required.
  • If the document is entirely new and does not have param descriptions, please add them
  • For some APIs, if we are ignoring them in exposing on API reference, why the APIs are actually public?

packages/calling/src/CallHistory/types.ts Outdated Show resolved Hide resolved
packages/calling/src/CallSettings/CallSettings.ts Outdated Show resolved Hide resolved
packages/calling/src/Contacts/ContactsClient.ts Outdated Show resolved Hide resolved
packages/calling/src/Contacts/ContactsClient.ts Outdated Show resolved Hide resolved
*
* Example - ContactGroup - https://github.com/webex/webex-js-sdk/wiki/Calling-Contacts#contactgroup
*/
createContactGroup(
Copy link
Contributor

Choose a reason for hiding this comment

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

Can mention param and descriptions.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

In class definitions we have it for the APIs, I feel in the interface we can avoid adding it

@mkesavan13
Copy link
Contributor

Also the PR title need not have the spark ticket ID since that is what is going to be shown in the commit history and would be public. The title shall look something like,

docs(calling): update tsdoc and api reference

@Kesari3008 Kesari3008 changed the title fix(calling-docs): Spark 428325 calling sdk api doc update fix(calling-docs): calling sdk api doc update Sep 26, 2023
Copy link
Contributor

@sreenara sreenara left a comment

Choose a reason for hiding this comment

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

Please see the comments

docs/index.html Outdated Show resolved Hide resolved
docs/index.html Show resolved Hide resolved
packages/calling/README.md Outdated Show resolved Hide resolved
packages/calling/src/CallHistory/CallHistory.ts Outdated Show resolved Hide resolved
packages/calling/src/CallHistory/types.ts Outdated Show resolved Hide resolved
packages/calling/src/CallSettings/CallSettings.ts Outdated Show resolved Hide resolved
packages/calling/src/CallSettings/CallSettings.ts Outdated Show resolved Hide resolved
packages/calling/src/Events/impl/index.ts Outdated Show resolved Hide resolved
packages/calling/src/api.ts Outdated Show resolved Hide resolved
Copy link
Contributor

@mkesavan13 mkesavan13 left a comment

Choose a reason for hiding this comment

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

Some of the comments apply to multiple places of similar scenarios. Kindly try to cover them as well

packages/calling/src/CallSettings/CallSettings.ts Outdated Show resolved Hide resolved
packages/calling/src/CallSettings/types.ts Show resolved Hide resolved
packages/calling/src/CallSettings/types.ts Show resolved Hide resolved
packages/calling/src/CallSettings/types.ts Outdated Show resolved Hide resolved
packages/calling/src/CallSettings/types.ts Outdated Show resolved Hide resolved
packages/calling/src/Contacts/types.ts Outdated Show resolved Hide resolved
Copy link
Contributor

@sreenara sreenara left a comment

Choose a reason for hiding this comment

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

LGTM!

Copy link
Contributor

@mkesavan13 mkesavan13 left a comment

Choose a reason for hiding this comment

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

Looks good. Approved

@Kesari3008 Kesari3008 merged commit b0a4ea9 into webex:next Oct 3, 2023
9 of 11 checks passed
Kesari3008 added a commit to Kesari3008/webex-js-sdk that referenced this pull request Oct 3, 2023
Co-authored-by: Priya Kesari <pkesari@cisco.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
validated If the pull request is validated for automation.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants