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

Fixes #37156 - Pin victory-core to 36.8.z #10043

Merged
merged 1 commit into from
Feb 14, 2024

Conversation

sjha4
Copy link
Contributor

@sjha4 sjha4 commented Feb 9, 2024

Pin victory-core to less than 36.9.0 which breaks react-charts. See patternfly/patternfly-react#10064

@sjha4 sjha4 requested a review from a team as a code owner February 9, 2024 18:23
@jeremylenz
Copy link
Contributor

Is Foreman the correct place for it? We only use it in Katello..

@jeremylenz jeremylenz requested a review from MariaAga February 9, 2024 18:36
@sjha4
Copy link
Contributor Author

sjha4 commented Feb 9, 2024

Is Foreman the correct place for it? We only use it in Katello..

I thought REX uses charts too..Let me check..

So I found it in these places in foreman project:

https://github.com/search?q=org%3Atheforeman%20%40patternfly%2Freact-charts&type=code

@sjha4
Copy link
Contributor Author

sjha4 commented Feb 12, 2024

@evgeni Does this look good?

package.json Outdated Show resolved Hide resolved
@@ -28,7 +28,8 @@
"surge",
"webpack-bundle-analyzer",
"tabbable",
"@adobe/css-tools"
"@adobe/css-tools",
"victory-core"
Copy link
Member

Choose a reason for hiding this comment

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

Why is it excluded?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I am not seeing this packaged today so not sure we want to package it..

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@evgeni Is this supposed to exclude only dev dependencies?

Copy link
Member

Choose a reason for hiding this comment

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

I'm unsure about this change, but the list must remain sorted.

Copy link
Member

@evgeni evgeni left a comment

Choose a reason for hiding this comment

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

the lists being sorted would be cool

@jeremylenz jeremylenz merged commit f3be1e5 into theforeman:develop Feb 14, 2024
31 of 33 checks passed
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.

5 participants