-
Notifications
You must be signed in to change notification settings - Fork 469
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
chore(deps): pin dependency graphql to 14.1.1 - autoclosed #1078
Conversation
d35cc5a
to
578f5a7
Compare
This follows up on @trevor-sheer's #951, which switched to exact pinning for all deps in the `apollo` CLI. As the #951 PR body notes, we want exactly that for `apollo`, however using the `rangeStrategy` of `pin` also includes renovation of the `engines` setting in `package.json`, which we don't want and resulted in #1078 which also exact-pinned `engines` for `node` and `npm`. (screenshot as it will change after this PR: https://c.jro.cc/ac9256ebf6ae). That PR didn't and won't land, but it's currently immortal and we want it to go away. It's existence, as it makes clear in the PR body, is defined by configuration though. Specifically, the default Renovate configuration on this repository uses the `renovate-config-apollo-open-source` defaults. i.e.: https://github.com/apollographql/renovate-config-apollo-open-source/blob/master/package.json That default specifies we extend `:pinOnlyDevDependencies`: https://github.com/apollographql/renovate-config-apollo-open-source/blob/master/package.json#L14 And that `:pinOnlyDevDependencies` is defined as: https://renovatebot.com/docs/presets-default/#pinonlydevdependencies This changes to the more appropriate `:pinAllExceptPeerDependencies`: https://renovatebot.com/docs/presets-default/#pinallexceptpeerdependencies ...which excludes `engines`.
This follows up on @trevor-sheer's #951, which switched to exact pinning for all deps in the `apollo` CLI. As the #951 PR body notes, we want exactly that for `apollo`, however using the `rangeStrategy` of `pin` also includes renovation of the `engines` setting in `package.json`, which we don't want and resulted in #1078 which also exact-pinned `engines` for `node` and `npm`. (screenshot as it will change after this PR: https://c.jro.cc/ac9256ebf6ae). I think @trevor-sheer tried to fix this with #889 and #1069, but I believe this would be the more complete solution to that configuration. The #951 which prompted this commit message didn't and won't land, but it's currently immortal and we want it to go away. It's existence, as it makes clear in the PR body, is defined by configuration though. Specifically, the default Renovate configuration on this repository uses the `renovate-config-apollo-open-source` defaults. i.e.: https://github.com/apollographql/renovate-config-apollo-open-source/blob/master/package.json That default specifies we extend `:pinOnlyDevDependencies`: https://github.com/apollographql/renovate-config-apollo-open-source/blob/master/package.json#L14 And that `:pinOnlyDevDependencies` is defined as: https://renovatebot.com/docs/presets-default/#pinonlydevdependencies This changes to the more appropriate `:pinAllExceptPeerDependencies`: https://renovatebot.com/docs/presets-default/#pinallexceptpeerdependencies ...which excludes `engines`.
This follows up on @trevor-sheer's #951, which switched to exact pinning for all deps in the `apollo` CLI. As the #951 PR body notes, we want exactly that for `apollo`, however using the `rangeStrategy` of `pin` also includes renovation of the `engines` setting in `package.json`, which we don't want and resulted in #1078 which also exact-pinned `engines` for `node` and `npm`. (screenshot as it will change after this PR: https://c.jro.cc/ac9256ebf6ae). I think @trevor-sheer tried to fix this with #889 and #1069, but I believe this would be the more complete solution to that configuration. The #951 which prompted this commit message didn't and won't land, but it's currently immortal and we want it to go away. It's existence, as it makes clear in the PR body, is defined by configuration though. Specifically, the default Renovate configuration on this repository uses the `renovate-config-apollo-open-source` defaults. i.e.: https://github.com/apollographql/renovate-config-apollo-open-source/blob/master/package.json That default specifies we extend `:pinOnlyDevDependencies`: https://github.com/apollographql/renovate-config-apollo-open-source/blob/master/package.json#L14 And that `:pinOnlyDevDependencies` is defined as: https://renovatebot.com/docs/presets-default/#pinonlydevdependencies This changes to the more appropriate `:pinAllExceptPeerDependencies`: https://renovatebot.com/docs/presets-default/#pinallexceptpeerdependencies ...which excludes `engines`.
578f5a7
to
d2610b8
Compare
@trevor-scheer I think we might actually want to land this pinning. Note that was not the case for this PR before I opened #1095, since this PR also tried to pin Right now — unless the user has installed |
6281f76
to
194be03
Compare
@abernix please feel at ease correcting me if I'm incorrect in saying any of this - but here's my experience. We do have a lot of users adding the CLI as a dev dependency. The result of pinning graphql in this package means that those users will likely end up with multiple versions if they've required graphql in their project...and we know how that goes. Granted, this does still require v14+ from our users, but that's more reasonable than requiring them to update their version of graphql as frequently as we update ours (in the case that we have it pinned). By leaving this unpinned, the CLI gets its v14+ which is sufficient, and our users can have whichever specific version of graphql they like >14 without any problems. Referencing some commentary that followed a recent previous pinning of graphql: All this is to say, my recommendation is to leave it unpinned (and specify this in our renovate config accordingly). However, I'm all ears if you've got some insight worth sharing on the matter 😄 |
194be03
to
c84944e
Compare
c84944e
to
40154f5
Compare
Per the correct assessment of what we want from the `graphql` package within the `apollo` package, in the following conversation: #1078 (comment) Specifically, we're okay using whatever 14.x version of `graphql` is potentially already available to us when someone installs `apollo` into their application's `devDependencies` in an existing application. If the existing application doesn't already have a 14.x version of `graphql` we will use the latest `14.x` version. With any success, this will cl_ose #1078 automatically (and not just because I said "close PR number" in this commit message!).
@trevor-scheer I think your stance here is correct. I've opened #1103 with what I believe to be the correct solution. Note, I'll go ahead and merge that and see if this PR gets automatically closed — as I expect to happen since the pinning of |
…`. (#1103) Per the correct assessment of what we want from the `graphql` package within the `apollo` package, in the following conversation: #1078 (comment) Specifically, we're okay using whatever 14.x version of `graphql` is potentially already available to us when someone installs `apollo` into their application's `devDependencies` in an existing application. If the existing application doesn't already have a 14.x version of `graphql` we will use the latest `14.x` version. With any success, this will cl_ose #1078 automatically (and not just because I said "close PR number" in this commit message!).
This PR contains the following updates:
^14.0.2
->14.1.1
📌 Important: Renovate will wait until you have merged this Pin PR before creating any upgrade PRs for the affected packages. Add the preset
:preserveSemverRanges
your config if you instead don't wish to pin dependencies.Renovate configuration
📅 Schedule: "after 6pm every weekday,before 5am every weekday" in timezone America/Los_Angeles.
🚦 Automerge: Disabled by config. Please merge this manually once you are satisfied.
♻️ Rebasing: Whenever PR is stale, or if you modify the PR title to begin with "
rebase!
".👻 Immortal: This PR will be recreated if closed unmerged. Get config help if that's undesired.
This PR has been generated by Renovate Bot. View repository job log here.