-
-
Notifications
You must be signed in to change notification settings - Fork 285
add an API option to ignore compat #1607
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
base: master
Are you sure you want to change the base?
Conversation
|
Generally best to avoid double negatives. How about |
|
You will only call this as |
Codecov Report
@@ Coverage Diff @@
## master #1607 +/- ##
==========================================
+ Coverage 86.44% 86.58% +0.13%
==========================================
Files 24 24
Lines 5387 5390 +3
==========================================
+ Hits 4657 4667 +10
+ Misses 730 723 -7
Continue to review full report at Codecov.
|
|
If we want to avoid double negatives, how about something like |
|
Nah, I guess this is ok. |
|
Bump. Is there any objection to merging this? Seems like it could be very useful. |
|
How hard would it be to (perhaps as an additional option) add a comprehensive set of warnings about what compats are being overridden? I think the ideal situation would not be to permanently just always go around ignoring compats, but to make it extremely simple to resolve specific issues. Seems this PR would get us halfway there, but the other half is transparency about what's happening. |
|
Not too hard, just go through all compat in the end and see what is incompatible. However, since I don't think that this should really be used I am personally not very interested in putting much more effort than this into it. |
|
What should this be used for? |
|
To second @cstjean, can you please elaborate on the use case for this? |
|
For people that complain about compat bounds. |
|
My concern is that after an upgrade that ignores compat bounds, they would complain about broken software instead. |
|
@tpapp commented on Aug 7, 2020, 5:14 PM GMT+4:30:
I have been using Python a lot, and I have yet to see problems out of ignored compat bounds. Yet Julia's strict insistence on compat bounds, coupled with badly defined compats by package authors, is giving me trouble almost 1 in 3 times I install a package. I think a major problem is that when the package authors don't specify a compat bound, e.g., see https://github.com/peterkovesi/PerceptualColourMaps.jl/blob/v0.3.0/REQUIRE, Pkg defaults to respecting their current used versions, and thus installing one package with old deps will cause a cascade of downgrades. My own philosophy is to always use the latest versions, and if sth breaks, stop using it or create a specific env for just that package. It's madness to do snowball-downgrading by default. See also my issue on LanguageServer.jl, where they state that they use ugly hacks just to circumvent this behavior. A much better approach to preserving compatibility between packages is to use Rich Hickey's philosophy; Don't change existing API in breaking ways. If you really need a breaking change, add a new API, and preserve the old one. Something as simple as |
It is hard to address this without specifics.
Note that this particular package does have compat bounds (since the modules it uses are standard libraries, compat bounds on
This has been discussed extensively on Github and Discourse, so I would rather not open that question here. Briefly, it is not the model Julia subscribes to. About this PR: my expectation is that instead of complaints about compat bounds, package maintainers would get bug reports about a broken state that would require tracking down problems that happen mostly in other packages (since that's where compat would be turned off). |
|
We DO suggest that packages follow semver like Julia itself does, which includes not making any breaking changes except in major versions. However, the reality of life is that people don't always do what you suggest they do. |
|
What if, when someone uses this feature, they get a warning message saying that it may break things, and it is their responsibility (not a package maintainer's) to solve any problems that arise when using this feature? |
|
@tpapp commented on Aug 7, 2020, 8:31 PM GMT+4:30:
Yes, I had mistaken the source of the issue. I edited my comment to point to the real culprit, but unfortunately, it doesn't get reflected in emails. (Is it a good practice to reply to comments instead of editing them to make sure people get the update in their email clients?)
Perhaps this issue will get dissolved after a year or two then. That (latest) problematic package that
I don't quite understand you here. Can you elaborate further? My impression is that ignoring compat bounds will mostly harm unmaintained packages that haven't updated their deps? A workaround for this problem might be to add a issue reporting helper tool to Pkg, that outputs all relevant info (OS version, Julia version, Python used by PyCall, ...), and include the ignored bounds in this output. When people open issues, this will be a clear hint that they have ignored bounds, and the author can ask them to try to reproduce under strict bounds. These kinds of stuff are standard practice in, e.g., reporting Emacs issues. The author will ask you to reproduce under a clean env ( |
|
I mostly did this PR to have something to point to when people complain that they are not getting the latest version. They can use this option and when everything breaks, they will reach enlightenment. |
Not necessarily. A maintained package may not update their
I wish it was so, but in practice I think some people will complain vocally. Being able to say "told you so" doesn't really help anyone in this situation. That said, if this is one of those pedagogical Zen PRs, it is a nice one. |
| for (name,vs) in cd | ||
| # check conflicts?? | ||
| all_compat_u_vr[name] = vs | ||
| if !ctx.ignore_compat |
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.
| if !ctx.ignore_compat | |
| if ctx.ignore_compat | |
| @warn "You are using the ignore_compat feature. Use of this feature may cause packages to break. It is the responsibility of the user (not the maintainers of any packages) to solve any problems that arise from the use of this feature." maxlog = 1 | |
| else |
|
Needs a rebase? |
Since this change is so small, I might as well probe what people think about this.
This adds an option that just turns off all compat (except the compat in your project) and gives you the latest versions of everything.
No docs yet because that will be the majority of the work for this PR and I want to see some opinions first.