-
Notifications
You must be signed in to change notification settings - Fork 1k
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
Unify development dependencies #5974
Unify development dependencies #5974
Conversation
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.
Looks like nothing breaks. 😀
The downside of this PR is that some dependencies are no longer locked in CI now. That means CI is less deterministic and potentially could get affected by issues in dependencies. For example, I guess there was a reason why But this may be acceptable. |
After a second look, the only dependency that got relaxed with these changes is |
6baf9e2
to
5384a2e
Compare
I was not too happy about the previous approach, because it was adding dependencies to the updater that it didn't really need. I changed the approach so that the set of dependencies used by It's probably a bit too hacky, but essentially the same thing other gemspecs are doing by pulling development dependencies from Let me know what you think! |
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.
This way of doing it makes sense to me, totally fine with the hack, since these two libs/apps are effectively joined at the hip into a monorepo.
79f4c93
to
b7e5587
Compare
b7e5587
to
8d9aede
Compare
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.
I think this is an okay approach. I agree with @jeffwidman that this is effectively a monorepo of core and updater anyway, so it makes sense to share dev dependencies.
I had one minor nit but it's not blocking
8d9aede
to
459c6ee
Compare
It worked! 🥳 |
Just curious to see what breaks if we unify our development dependencies.