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.

Sign up for GitHub

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

Jump to bottom

feat: upgrade-to-yarn-remove-node-modules-folder #1438

Merged
merged 4 commits into from
Oct 14, 2024
Merged

feat: upgrade-to-yarn-remove-node-modules-folder #1438

merged 4 commits into from
Oct 14, 2024

Conversation

Shreyas281299
Copy link
Contributor

@Shreyas281299 Shreyas281299 commented Oct 8, 2024 •
edited
Loading

Completes -SPARK-559074

My suggestion on how to review this PR - close the packages folder, it only has file name changes. After that its a small PR

This PR is the first in the series of many PRs to create react-widgets a yarn workspace

This PR completes the first two tasks in the list bellow

Tasks required to make react-widgets a yarn workspace

  • Move components out of node_modules
  • Adjust the tooling to use the new paths
  • Enable yarn workspace in react-widget
  • Add dependencies inside each package
  • Add scripts for each package
  • Fix tooling to use yarn workspaces packages

Checks

  • Build is passing
  • UTs are passing
  • Integration tests are passing

Testing

  • Ran build:all
  • Served all the demos
  • Uts are passing.

Issues found

All these errors are persistent in master and node-upgrade branch also

  • People list has ENOENT
 error while building - any css file thats why this error - its doesnt affect anything

  • Widget-meet has ENOENT - any css file thats why this error - its doesnt affect anything

  • All demo components fails the buildES function.
  • Issue with tsconfig- webpack build fails for all ts 'widget' components
  • Failling command
- npm run deploy is failing in master and node-upgrade as well
  • We are ignoring all the src files for eslint tests
  • journey tests are failling in the node-upgrade branch as well

Shreyas281299
Shreyas281299 commented Oct 8, 2024
View reviewed changes
package-lock.json Outdated Show resolved Hide resolved
Shreyas281299
Shreyas281299 commented Oct 8, 2024
View reviewed changes
.eslintignore Outdated Show resolved Hide resolved
@Shreyas281299 Shreyas281299 mentioned this pull request Oct 9, 2024
Closed
9 tasks
mkesavan13
mkesavan13 approved these changes Oct 10, 2024
View reviewed changes
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.

Mostly looks good. Just one question. Approving

packages/@webex/react-component-adaptive-card/package.json Show resolved Hide resolved
@mkesavan13
Copy link
Contributor

Should we re-word the PR title?

@Shreyas281299 Shreyas281299 changed the title feat: move-components-out-of-node-modules feat: upgrade-to-yarn-remove-node-modules-folder Oct 10, 2024
@Shreyas281299
Copy link
Contributor Author

Should we re-word the PR title?

Updated

Kesari3008
Kesari3008 approved these changes Oct 11, 2024
View reviewed changes
Copy link
Contributor

@Kesari3008 Kesari3008 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

packages/@webex/webex-sign-in-page/package.json Show resolved Hide resolved
packages/@webex/webex-widget-base/package.json Show resolved Hide resolved
packages/@webex/widget-call-history/package.json Show resolved Hide resolved
packages/@webex/widget-number-pad/package.json Show resolved Hide resolved
packages/@webex/widget-speed-dial/package.json Show resolved Hide resolved
packages/@webex/widget-voice-mail/package.json Show resolved Hide resolved
scripts/build/commands/dist.js Outdated Show resolved Hide resolved
scripts/utils/build.js Show resolved Hide resolved
@Shreyas281299 Shreyas281299 merged commit 4cbd7b3 into webex:node-upgrade Oct 14, 2024
1 check failed
@Shreyas281299 Shreyas281299 mentioned this pull request Oct 14, 2024
Closed
9 tasks
@Shreyas281299 Shreyas281299 mentioned this pull request Oct 23, 2024
Merged
9 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Kesari3008 Kesari3008 Kesari3008 approved these changes

@mkesavan13 mkesavan13 mkesavan13 approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@Shreyas281299 @mkesavan13 @Kesari3008 @ShreyasSharma28