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 lambdasoup dependency #346

Closed
wants to merge 1 commit into from
Closed

Conversation

maxRN
Copy link
Contributor

@maxRN maxRN commented Sep 6, 2024

Fixes #324

@aantron
Copy link
Owner

aantron commented Sep 6, 2024

Thanks! I typically don't bump in a library for bugfixes, because, on the one hand, upgrading everything will transparently pull in Lambda Soup 1.1.1 and fix the bug for users, and on the other hand, someone might have an unexpected reason why they need a lower version of Lambda Soup, or something else that needs a lower version of Lambda Soup, or who knows what kind of dependency tangle, and experience suggests that it's best to avoid it. And this bug might not even affect "most" users of Dream, including someone that might end up with an unpredictable dep tangle. So for Dream (and Lwt, etc.) I think it's better to have a sort of "loose binding" type of situation, where if someone runs into this, the answer is just to upgrade Lambda Soup (independently of Dream), and most people won't run into it, because opam/whatever package client will just install the right thing anyway.

Would definitely bump in an app or more leaf-like library, or would bump for breaking change where Dream wouldn't even build against an older version, or would be horrendously broken without the bump.

@aantron
Copy link
Owner

aantron commented Sep 6, 2024

For example, if an absolutely critical bug was found in http/af, and it would affect all or most users of Dream, even if at first they would not spot it, that would be worth a bump.

@aantron
Copy link
Owner

aantron commented Sep 6, 2024

Also, for comparison, I decided not to bump ctypes in Luv for aantron/luv#159, because even though that fix reduces warnings on ctypes 0.23.0 at the cost of increasing warnings on ctypes < 0.23.0, for anyone for whom these are only warnings (not GCC 14), it's not worth risking a rigid dependency tangle which would artificially force them to do something. It just allows them more breathing room before they have to make decisions about what to actually upgrade, drop, etc. Only people who are stuck with ctypes 0.22 for some reason and must use GCC 14 have to suffer :) And then we might be able to fix their issue in another way :) But if I did the bump, anyone using ctypes 0.22, whether or not using GCC 14, would suffer.

@aantron
Copy link
Owner

aantron commented Sep 6, 2024

So in summary, I think this is a case where it's best for Dream to remain "transparent" w.r.t. the dep and just pass it through.

@maxRN
Copy link
Contributor Author

maxRN commented Sep 11, 2024

Thanks being so open about your thought process, definitely makes sense to not bump the version. I'm still often stuck thinking in "app-land" and not "library-land", so for me it feels wrong to not bump a version that has a known bug.

@maxRN maxRN closed this Sep 11, 2024
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.

HTML Attributes that have a : in the middle of the name truncate everything before the :
2 participants