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

[cuix] Changes made in local Hue after updating the CUIX library #3811

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

ananya-agarwal
Copy link
Collaborator

@ananya-agarwal ananya-agarwal commented Aug 6, 2024

What changes were proposed in this pull request?

This PR contains the files changed in the local system after updating CUIX library.
Scope Doc: https://docs.google.com/document/d/1Fa2Dj5YGedp60sVTMKKC4ADljxweJkX5jnJ-h-MKG1s/edit

Steps in the Readme.md of: https://github.infra.cloudera.com/balm/cuix-hue
regarding how to update CUIX library.

How was this patch tested?

  1. Hue working on the local system: Sanity checking done for editors like Hive, etc. Metrics (the (merged) updated metrics tab of Admin page uses components of CUIX) works fine on this branch.
  2. Badge for example (/Users/ananya.agarwal/hue/node_modules/cuix/dist/components/Badge/Badge.d.ts) wasn't present in local system because it was updated on the CUIX some months ago. After updating CUIX, it is now present in the local system.

Please review Hue Contributing Guide before opening a pull request.

Comment on lines +7357 to +7052
"@dnd-kit/core": "6.1.0",
"@dnd-kit/modifiers": "7.0.0",
"@dnd-kit/sortable": "8.0.0",
"@fullstory/browser": "1.6.2",
"@react-hookz/web": "^23.1.0",
"compare-versions": "5.0.3",
"dompurify": "2.0.11",
"javascript-autocomplete": "1.0.5",
"js-cookie": "2.2.1",
"moment-timezone": "0.5.31",
"popper.js": "1.16.0",
"preact": "10.11.0"
"preact": "10.11.0",
"react-redux": "8.0.5",
"recharts": "2.1.9",
"redux": "4.0.5",
"redux-saga": "1.1.3",
"styled-components": "6.1.8",
"styled-theming": "2.2.0"
Copy link
Contributor

Choose a reason for hiding this comment

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

@bjornalm these new dependencies seems to break Hue, when you did the initial include of cuix did you also remove these somehow?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Some of them are new, and they introduce another dependency on an older version of react itself. This dependency is not resolved by dedupe so I've actually tried to just remove them from cuix since I'm pretty sure we don't need them for the components we are using. This seem to work, failing unit tests become green again, and I'll create a new PR with those modifications.

Copy link
Collaborator

@tabraiz12 tabraiz12 left a comment

Choose a reason for hiding this comment

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

LGTM, review from @bjornalm required as it uses his script to copy.

Copy link

This PR is stale because it has been open 45 days with no activity and is not labeled "Prevent stale". Remove "stale" label or comment or this will be closed in 10 days.

@github-actions github-actions bot added the Stale label Oct 21, 2024
@bjornalm bjornalm removed the Stale label Oct 21, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants