-
Notifications
You must be signed in to change notification settings - Fork 12
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
Upgrade underscore, d3-color, debug, cosmiconfig, yaml dependencies #217
Conversation
fbe8f6d
to
e692cac
Compare
package.json
Outdated
@@ -24,7 +24,7 @@ | |||
"@emotion/css": "^11.1.3", | |||
"@grafana/aws-sdk": "0.3.0", | |||
"@grafana/data": "9.3.2", | |||
"@grafana/e2e": "9.3.2", | |||
"@grafana/e2e": "10.2.0", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
what's the reason for updating just the e2e package separately here? is it to do with the underscore package?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, first I tried to update @grafana/e2e to see if the underscore package dependency would be higher. It was not, but I did not revert the upgrade.
I didn't necessarily revert some other upgrades which didn't help. In this case I thought @grafana/e2e would be a harmless upgrade if it was only used in e2e tests.
Is this a case where you recommend that I leave it at its original version? Let me know!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
actually, it doesn't even look like there are any e2e tests and the e2e and e2e-selectors packages aren't even used in x-ray. we could just remove those deps as we'll eventually be moving to the updated e2e testing package later on.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
do you have some recommendation to check for dep usage? for example could i just find "grafana/e2e" by text in the files?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also I removed the e2e-selectors dependency and the scripts for e2e, if you could just confirm that is ok!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@kevinwcyu I'm gonna go with this and merge, let me know if in hindsight it was not OK!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@fridgepoet Sorry, i missed your comments. I just search for the package name in the files to see if we use it. Also removing the scripts is fine since we don't actually use them.
What this PR does / why we need it:
Similar to grafana/iot-sitewise-datasource#249
This PR upgrades cspell, fork-ts-checker-webpack-plugin and add cosmiconfig resolution in order to upgrade the package yaml version. The main reason the older version of yaml was used was due to a transitive dependency of an older version of cosmiconfig.
fork-ts-checker-webpack-plugin and cspell were on older versions requiring an older version of cosmiconfig.
fork-ts-checker-webpack-plugin and cspell were updated, but finally @emotion/css still used an older version of cosmiconfig. So the latest version of cosmiconfig was added to resolutions.
Also there was a @grafana/e2e dependency which was unused. Other references to e2e were also removed.
Which issue(s) this PR fixes:
Fixes https://github.com/grafana/oss-plugin-partnerships/issues/539
Fixes https://github.com/grafana/oss-plugin-partnerships/issues/541
Fixes https://github.com/grafana/oss-plugin-partnerships/issues/537
Fixes https://github.com/grafana/oss-plugin-partnerships/issues/535
Special notes for your reviewer: