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

Bump deps including extreme #5

Merged
merged 3 commits into from
Jan 31, 2024
Merged

Bump deps including extreme #5

merged 3 commits into from
Jan 31, 2024

Conversation

svrdlans
Copy link
Contributor

fixes issue that occurred in agora caused by a bug in extreme library addressed with this PR

@svrdlans svrdlans requested review from tonyvanriet and a team January 31, 2024 17:02
@tonyvanriet
Copy link
Member

I think we need to change the extreme dep constraint in mix.exs to ensure host applications use the fixed version.

maybe ~> 1.0 and >= v1.0.5 ?

Copy link
Member

@tonyvanriet tonyvanriet left a comment

Choose a reason for hiding this comment

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

dep constraint

@svrdlans
Copy link
Contributor Author

shouldn't >= 1.0.5 suffice?

@svrdlans
Copy link
Contributor Author

although I do believe bumping kelvin in harness libs and (unharnessed) apps will give us the desired effect

@svrdlans svrdlans requested a review from tonyvanriet January 31, 2024 17:42
@tonyvanriet
Copy link
Member

yes, just running deps.update in the host app needs to be done and will update extreme, I think it's important for the dependency tree to actually try to prevent people from using < 1.0.5.

if it's only >= 1.0.5, then it would allow deps.update to extreme 2.x even though that would include breaking changes, in theory. I think the ~> 1.0 is important for ensuring any move to 2.x is done deliberately.

all ears for arguments against this, but that's my take.

@svrdlans
Copy link
Contributor Author

ah, good point, I was thinking 2.0 is so far away from us (if it comes at all), that we don't need to worry at all

but definitely makes sense to not let it be accidental

@svrdlans svrdlans merged commit 9e3f1f8 into main Jan 31, 2024
2 checks passed
@svrdlans svrdlans deleted the bump-deps-including-extreme branch January 31, 2024 18:21
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