-
Notifications
You must be signed in to change notification settings - Fork 22
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
Disable AvailabilityTest checking for skipping install of packages #1037
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #1037 +/- ##
==========================================
+ Coverage 74.37% 74.48% +0.11%
==========================================
Files 55 55
Lines 4534 4527 -7
==========================================
Hits 3372 3372
+ Misses 1162 1155 -7
|
66ac001
to
775b09d
Compare
We verified locally, that this PR gets rid of the |
Ahhh, that makes sense. But I think there was a reason for the added code: to avoid network issues... considering this:
So each And if I disconnect my laptop from the internet, it gets suck on this for a long time. So won't we be treading one source of OSCAR startup issues for another? Of course one might be more or less common than the other... Another problem is that Unfortunately I again did not end up having time to work towards the long-planned better solution, which is to provide |
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 this fixes more problems right now than it causes, go ahead, but we need to eventually solve the "no network connection" issues
When I talked to @ThomasBreuer yesterday, our primary goal was to get oscar-system/Oscar.jl#3688 merged, for the simple reason that after that we again have OscarCI that reports potential problems early. |
The point is that just calling the |
(I don't have the right to proceed with merging and tagging a release of this.) |
superceded by #1039 |
In some edge-cases a package is available, but one of its transitive dependencies is not. This piece of code would have skipped any installation in this case.
We (@ThomasBreuer and I) ran into this in oscar-system/Oscar.jl#3688, because
recog
is available, but its transitive dependencyio
needs compilation.