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

Pin all apollo deps via renovate config #951

Merged
merged 1 commit into from
Jan 31, 2019

Conversation

trevor-scheer
Copy link
Member

@trevor-scheer trevor-scheer commented Jan 31, 2019

Based on recent experience with packages breaking underneath us, and the typical inherent usage of the CLI being via npx, we should pin all deps for the apollo package.

For reference, see example 3 in the link below. This decision is also recommended by renovate for our case:
https://renovatebot.com/docs/dependency-pinning/#so-whats-best

Copy link
Member

@hwillson hwillson left a comment

Choose a reason for hiding this comment

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

Looks great to me @trevor-scheer - thanks!

@trevor-scheer trevor-scheer merged commit b062c70 into master Jan 31, 2019
@trevor-scheer trevor-scheer deleted the trevor/pin-apollo-deps branch January 31, 2019 18:37
@trevor-scheer trevor-scheer changed the title Update renovate config for apollo package Pin all apollo deps via renovate config Jan 31, 2019
abernix added a commit that referenced this pull request Mar 8, 2019
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`.
abernix added a commit that referenced this pull request Mar 8, 2019
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`.
abernix added a commit that referenced this pull request Mar 8, 2019
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`.
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.

2 participants