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

Removes UI Framework KUI doc site #1379

Merged
merged 1 commit into from
Apr 15, 2022
Merged

Removes UI Framework KUI doc site #1379

merged 1 commit into from
Apr 15, 2022

Conversation

tmarkley
Copy link
Contributor

@tmarkley tmarkley commented Mar 24, 2022 •
edited
Loading

Description

  • KUI components are no longer being developed and the dependencies for the doc site are related to CVEs.
  • Removes related grunt tasks.
  • Removes all unused dependencies from the osd/ui-framework package.

Issues Resolved

Resolves #1096

Check List

  • New functionality includes testing.
    • All tests pass
      • yarn test:jest
      • yarn test:jest_integration
      • yarn test:ftr
  • New functionality has been documented.
  • Commits are signed per the DCO using --signoff

@tmarkley tmarkley added dependencies Pull requests that update a dependency file v2.0.0 ui library Issue or PR related to the UI component library cve Security vulnerabilities detected by Dependabot or Mend labels Mar 24, 2022
@tmarkley tmarkley requested a review from a team as a code owner March 24, 2022 01:34
ashwin-pc
ashwin-pc reviewed Mar 24, 2022
View reviewed changes
Copy link
Member

@ashwin-pc ashwin-pc left a comment

Choose a reason for hiding this comment

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

The rest looks good. Just a question about removing the grunt tasks to compile the scss for ui-framework

packages/osd-ui-framework/Gruntfile.js Show resolved Hide resolved
packages/osd-ui-framework/Gruntfile.js Show resolved Hide resolved
@tmarkley tmarkley requested a review from ashwin-pc March 24, 2022 22:20
@tmarkley
Copy link
Contributor Author

I successfully ran the yarn uiFramework:compileCss script but it removed all of the Webkit CSS extensions from the light and dark compiled CSS, so I reverted those changes:

$ yarn uiFramework:compileCss
yarn run v1.22.17
$ cd packages/osd-ui-framework && yarn compileCss
$ grunt compileCss
Running "compileCss" task

Done.
Done in 2.67s.

@ashwin-pc
Copy link
Member

I successfully ran the yarn uiFramework:compileCss script but it removed all of the Webkit CSS extensions from the light and dark compiled CSS, so I reverted those changes:

Ran that on main, looks like that's unrelated to your change. The rest looks good to me!

ashwin-pc
ashwin-pc previously approved these changes Mar 25, 2022
View reviewed changes
@tmarkley tmarkley requested a review from ashwin-pc March 25, 2022 19:08
boktorbb
boktorbb previously approved these changes Mar 25, 2022
View reviewed changes
@kavilla
Copy link
Member

kavilla commented Apr 5, 2022

Looks like this doesn't actually had an impact on the CVE and the CVE was addressed. Given that has conflicts and the CVE was addressed I will remove the 2.0.0 label.

@kavilla kavilla added v3.0.0 and removed v2.0.0 labels Apr 5, 2022
@tmarkley
Copy link
Contributor Author

tmarkley commented Apr 11, 2022 •
edited
Loading

Looks like this doesn't actually had an impact on the CVE and the CVE was addressed. Given that has conflicts and the CVE was addressed I will remove the 2.0.0 label.

@kavilla the CVE was not addressed. highlight.js@9.18.5 is still in the yarn.lock file: https://github.com/opensearch-project/OpenSearch-Dashboards/blob/main/yarn.lock

@kavilla
Copy link
Member

kavilla commented Apr 11, 2022

Looks like this doesn't actually had an impact on the CVE and the CVE was addressed. Given that has conflicts and the CVE was addressed I will remove the 2.0.0 label.

@kavilla the CVE was not addressed. highlight.js@9.18.5 is still in the yarn.lock file: https://github.com/opensearch-project/OpenSearch-Dashboards/blob/main/yarn.lock

Apologies, when I checked the CVE it was automatically resolved but I guess it re-opened 4 days ago. I see there are conflicts here.

@tmarkley tmarkley added v2.0.0 and removed v3.0.0 labels Apr 12, 2022
@tmarkley tmarkley requested a review from boktorbb April 13, 2022 15:49
boktorbb
boktorbb previously approved these changes Apr 13, 2022
View reviewed changes
@kavilla
Copy link
Member

kavilla commented Apr 13, 2022

Similar with the other PR with the lock file being changed do we see any potential of breakage with plugins? Grant it is the lock file although we do is for handling a lot of our resolutions.

Thanks!

@tmarkley
Copy link
Contributor Author

Similar with the other PR with the lock file being changed do we see any potential of breakage with plugins? Grant it is the lock file although we do is for handling a lot of our resolutions.

Thanks!

Yeah I think there could be some risk here with the amount of dependencies that we're removing. I'm okay to hold off until next week and merge after RC1.

kavilla
kavilla previously requested changes Apr 14, 2022
View reviewed changes
Copy link
Member

@kavilla kavilla left a comment

Choose a reason for hiding this comment

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

Blocking for rc1 release.

@tmarkley tmarkley dismissed kavilla’s stale review April 15, 2022 16:36

We need to merge these CVE PRs in for RC1 to address any breaking changes.

Removes UI Framework KUI doc site
460bb68 
* KUI components are no longer being developed and the dependencies for
  the doc site are related to CVEs.
* Refactors grunt tasks for compiling KUI CSS.
* Removes doc site related grunt tasks.
* Removes all unused dependencies from the osd/ui-framework package.

### Issues
Prerequisite for resolving opensearch-project#1096 (along with the EUI upgrade)

Signed-off-by: Tommy Markley <markleyt@amazon.com>
@tmarkley tmarkley mentioned this pull request Apr 15, 2022
Closed
@tmarkley tmarkley merged commit f7a6754 into opensearch-project:main Apr 15, 2022
@tmarkley tmarkley deleted the highlightjs branch April 16, 2022 16:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ashwin-pc ashwin-pc ashwin-pc approved these changes

@boktorbb boktorbb boktorbb approved these changes

@AMoo-Miki AMoo-Miki Awaiting requested review from AMoo-Miki AMoo-Miki is a code owner

@ananzh ananzh Awaiting requested review from ananzh ananzh is a code owner

@kavilla kavilla Awaiting requested review from kavilla kavilla is a code owner

Assignees
No one assigned
Labels
cve Security vulnerabilities detected by Dependabot or Mend dependencies Pull requests that update a dependency file ui library Issue or PR related to the UI component library v2.0.0
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

WS-2020-0208 (Medium) detected in highlight.js-9.18.5.tgz
4 participants
@tmarkley @ashwin-pc @kavilla @boktorbb