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(meetings): fix types #3602

Merged
merged 5 commits into from
Jun 5, 2024

Conversation

Shreyas281299
Copy link
Contributor

@Shreyas281299 Shreyas281299 commented May 15, 2024

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

This pull request addresses

Export important classes, objects, and constants in plugin-meetings. This will make sure we dont have to import them from from '/dist' folder in WWC.

In internal-plugin-voicea, we were not creating declaration files.

by making the following changes

  • Exporting all required objects in plugin-meetings.
  • Changed commands in plugin-voicea, and added tsconfig to create proper declaration files.

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

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.

@Shreyas281299 Shreyas281299 requested a review from a team as a code owner May 15, 2024 04:53
@Shreyas281299 Shreyas281299 added the validated If the pull request is validated for automation. label May 15, 2024
@Shreyas281299
Copy link
Contributor Author

Code refactor of util.ts and utilv2.ts was required because with the existing code we were getting an empty declaration files.

@sreenara
Copy link
Contributor

Code refactor of util.ts and utilv2.ts was required because with the existing code we were getting an empty declaration files.

Please mark this comment next to the place where the refactor was done

if (type === _LOCUS_ID_) {
if (!(destination && destination.url)) {
throw new ParameterError('You cannot create a meeting by locus without a locus.url defined');
/**
Copy link
Contributor Author

@Shreyas281299 Shreyas281299 May 23, 2024

Choose a reason for hiding this comment

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

The refactor of util.ts is required as without this refactor an empty declaration file was being created.

For the refactor

  1. Created a class MeetingInfoUtil
  2. All the methods inside the class are static. No src code changes are made.

Copy link
Contributor

Choose a reason for hiding this comment

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

Thanks for this comment. I like it :-)

const MeetingInfoUtil: any = {};
const meetingInfoError =
'MeetingInfo is fetched with the meeting link, SIP URI, phone number, Hydra people ID, or a conversation URL.';
/**
Copy link
Contributor Author

@Shreyas281299 Shreyas281299 May 23, 2024

Choose a reason for hiding this comment

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

The refactor of utilv2.ts is required as without this refactor an empty declaration file was being created.

For the refactor

  1. Create a class MeetingInfoUtil
  2. All the methods inside the class are static. No src code changes are made.

@@ -41,8 +42,9 @@
"sinon": "^9.2.4"
},
"scripts": {
"build": "yarn build:src",
"build:src": "webex-legacy-tools build -dest \"./dist\" -src \"./src\" -js -ts -maps",
"ls": "pwd",
Copy link
Collaborator

Choose a reason for hiding this comment

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

@Shreyas281299 why do we want to add the ls command here ?
Also ls is doing pwd ? pwd is to get full path name of your current directory but ls is for listing files.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@Neeraj-swarnkar thanks. No this was added by mistake. I was testing something thats why I added this. Removing it now.

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.

Just had one question. Changes look fine. Let's get this reviewed by the web client team as well.

packages/@webex/internal-plugin-voicea/package.json 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.

Just one comment. Rest looks good to me :-)

packages/@webex/plugin-meetings/package.json Show resolved Hide resolved
if (type === _LOCUS_ID_) {
if (!(destination && destination.url)) {
throw new ParameterError('You cannot create a meeting by locus without a locus.url defined');
/**
Copy link
Contributor

Choose a reason for hiding this comment

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

Thanks for this comment. I like it :-)

Copy link
Collaborator

@Neeraj-swarnkar Neeraj-swarnkar 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.

@Shreyas281299 Shreyas281299 merged commit 4cfb88b into webex:next Jun 5, 2024
10 checks passed
@Shreyas281299 Shreyas281299 deleted the fix-meetings-voicea-types branch August 6, 2024 08:23
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.

6 participants