Skip to content

deps: ignore dependabot for peer dependencies#904

Merged
agilgur5 merged 1 commit intojaredpalmer:masterfrom
agilgur5:deps-dont-update-peers
Oct 14, 2020
Merged

deps: ignore dependabot for peer dependencies#904
agilgur5 merged 1 commit intojaredpalmer:masterfrom
agilgur5:deps-dont-update-peers

Conversation

@agilgur5
Copy link
Copy Markdown
Collaborator

@agilgur5 agilgur5 commented Oct 14, 2020

Description

  • several of the dependencies in the tree are only there because they
    are peerDeps, and not because we directly depend on them

    • they should only be upgraded in tandem with the dep(s) they are
      peers of and pretty much never in isolation
  • Greenkeeper made several PRs for peers which previously caused an
    erroneous/buggy update of @types/jest to 25 many months before Jest
    was upgraded to 25

    • and honestly those Greenkeeper PRs for peers had confused me a
      good deal too

Tags

Let's close some of the confusing Greenkeeper peer update PRs now that this has been configured:

Review Notes

I straight copy+pasted the eslint-config-react-app peerDeps from its package.json, so they're alphabetized the same way and shouldn't have any spelling mistakes.

- several of the dependencies in the tree are only there because they
  are peerDeps, and not because we directly depend on them
  - they should only be upgraded in tandem with the dep(s) they are
    peers of and pretty much never in isolation

- Greenkeeper made several PRs for peers which previously caused an
  erroneous/buggy update of `@types/jest` to 25 many months before Jest
  was upgraded to 25
  - and honestly those Greenkeeper PRs for peers had confused me a
    good deal too
@vercel

This comment has been minimized.

@agilgur5 agilgur5 added scope: dependencies Pull requests that update a dependency file scope: internal Changes only affect the internal API labels Oct 14, 2020
Copy link
Copy Markdown
Collaborator Author

@agilgur5 agilgur5 left a comment

Choose a reason for hiding this comment

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

peerDeps all look to line up to me, but I think tslib should probably be included in here too because of #719 (comment).

But somewhat problematically, it doesn't seem to have a peerDep on typescript, meaning one wouldn't get a peerDep mismatch error if an incorrect version were installed. And that means it could be easy to forget to check/upgrade tslib in tandem with typescript... Hmmm, need to think about that.

This might be unintentional, so should check upstream in tslib and perhaps file a PR.

@agilgur5
Copy link
Copy Markdown
Collaborator Author

Seems like microsoft/tslib#22 exists that goes into this. microsoft/tslib#22 (comment) seemed against using peerDeps but without an example that was difficult to understand how the use-case actually plays out (does it mean in the dependency tree or a single package?)

@agilgur5
Copy link
Copy Markdown
Collaborator Author

Since older versions of tslib seem to be forward-compatible, it seems like the decision to upgrade tslib would be independent of TS upgrade as a result, so might not make sense to ignore it then.

Added a comment in the thread upstream, but think this is good to go as is due to above. Can always add more to the list as well.

@agilgur5 agilgur5 merged commit 569c3ed into jaredpalmer:master Oct 14, 2020
@agilgur5
Copy link
Copy Markdown
Collaborator Author

agilgur5 commented Oct 14, 2020

Cleaned up ~74 Greenkeeper branches associated with @typescript-eslint/eslint-plugin, @typescript-eslint/parser, eslint-plugin-flowtype, eslint-plugin-import, eslint-plugin-react, eslint-plugin-react-hooks, @types/jest, and jest-watch-typeahead.

Think nearly all the rest can be cleaned up now too (~44 left). I think they're mostly minor version upgrades (hence no PR) or devDeps or both. Has taken a while to get here, have cleaned up 100s of Greenkeeper branches 😕

@agilgur5
Copy link
Copy Markdown
Collaborator Author

Deleted another ~12 Greenkeeper branches. They were mostly patch bumps of devDeps actually, a few minors and a few prod. Remaining ones seem to be all Prettier/eslint-config-prettier/eslint-plugin-prettier deps or template version bumps in the example dir

imgbot bot pushed a commit to mubaidr/tsdx that referenced this pull request Oct 31, 2020
- several of the dependencies in the tree are only there because they
  are peerDeps, and not because we directly depend on them
  - they should only be upgraded in tandem with the dep(s) they are
    peers of and pretty much never in isolation

- Greenkeeper made several PRs for peers which previously caused an
  erroneous/buggy update of `@types/jest` to 25 many months before Jest
  was upgraded to 25
  - and honestly those Greenkeeper PRs for peers had confused me a
    good deal too
paul-vd pushed a commit to EezyQuote/tsdx that referenced this pull request Dec 1, 2020
- several of the dependencies in the tree are only there because they
  are peerDeps, and not because we directly depend on them
  - they should only be upgraded in tandem with the dep(s) they are
    peers of and pretty much never in isolation

- Greenkeeper made several PRs for peers which previously caused an
  erroneous/buggy update of `@types/jest` to 25 many months before Jest
  was upgraded to 25
  - and honestly those Greenkeeper PRs for peers had confused me a
    good deal too
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

scope: dependencies Pull requests that update a dependency file scope: internal Changes only affect the internal API

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant