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

update peer dependency for mui/icons in material-renderers #2186

Merged
merged 2 commits into from
Oct 6, 2023
Merged

update peer dependency for mui/icons in material-renderers #2186

merged 2 commits into from
Oct 6, 2023

Conversation

IhsenBen
Copy link
Contributor

@IhsenBen IhsenBen commented Oct 6, 2023

self explanatory minor change

@netlify
Copy link

netlify bot commented Oct 6, 2023

Deploy Preview for jsonforms-examples ready!

Name Link
🔨 Latest commit cac6fd5
🔍 Latest deploy log https://app.netlify.com/sites/jsonforms-examples/deploys/6520050f3bef2600086809a8
😎 Deploy Preview https://deploy-preview-2186--jsonforms-examples.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

@CLAassistant
Copy link

CLAassistant commented Oct 6, 2023

CLA assistant check
All committers have signed the CLA.

Copy link
Contributor

@lucas-koehler lucas-koehler left a comment

Choose a reason for hiding this comment

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

Thanks for the contribution ❤️
I added a comment inline. Please have a look.

@@ -87,7 +87,7 @@
"@emotion/styled": "^11.3.0",
"@jsonforms/core": "3.1.1-alpha.0",
"@jsonforms/react": "3.1.1-alpha.0",
"@mui/icons-material": "~5.11.16",
"@mui/icons-material": ">=5.11.16",
Copy link
Contributor

Choose a reason for hiding this comment

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

>= seems a bit too lax to me because it also allows all higher major versions (e.g. 6.x, 7.x). I think we should change it to ^5.11.16 to allow all 5.x versions but no other major versions. If it's known to work with 6.x it could then be adapted to ^5.11.16 || ^6.0.0

Copy link
Contributor Author

Choose a reason for hiding this comment

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

it's done :D

@coveralls
Copy link

coveralls commented Oct 6, 2023

Coverage Status

coverage: 83.924% (-0.5%) from 84.469% when pulling cac6fd5 on IhsenBen:bug-fix/renderers-peerPkg-version into 28eeeca on eclipsesource:master.

Copy link
Contributor

@lucas-koehler lucas-koehler left a comment

Choose a reason for hiding this comment

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

Hi @IhsenBen ,
I changed it to ^5.11.16 because v6 does not exist yet and we don't know whether it will be compatible. Thanks for the issue and the PR :)

@IhsenBen
Copy link
Contributor Author

IhsenBen commented Oct 6, 2023

Hi @IhsenBen , I changed it to ^5.11.16 because v6 does not exist yet and we don't know whether it will be compatible. Thanks for the issue and the PR :)

no problem, thx for your reactivity.
have a nice day

@lucas-koehler
Copy link
Contributor

@IhsenBen could you sign the contributor license agreement please? Otherwise I cannot merge this. You won't need to do this again in case you open further PRs in the future :)

@IhsenBen
Copy link
Contributor Author

IhsenBen commented Oct 6, 2023

@IhsenBen could you sign the contributor license agreement please? Otherwise I cannot merge this. You won't need to do this again in case you open further PRs in the future :)

done

@lucas-koehler lucas-koehler merged commit 694020b into eclipsesource:master Oct 6, 2023
8 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.

material-renderers require strict outdated version of @mui/icons-material
4 participants