-
-
Notifications
You must be signed in to change notification settings - Fork 5.5k
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
Delete buggy stat(::Integer)
method
#54855
base: master
Are you sure you want to change the base?
Conversation
This is specifically tested for, hence the tests fail. What did triage agree to yesterday? I can modify #51711 accordingly. |
I opened this PR while screen sharing during triage, but it wasn't explicitly a triage decision. I just want to see what nanosoldier has to say about this. @nanosoldier I think the problem with |
The package evaluation job you requested has completed - possible new issues were detected. |
julia> fd(open("tmp", "w"))
23 |
The docs don't indicate that
|
Of the 19 packages that failed nano-soldier only on this version, 14 were false positives, 3 were due to There is perhaps still hope for this in 1.x |
Actually, the docs don't indicate at all what
|
Just came across this.
|
What did you expect? |
False is a file for me lol
|
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 good to me esp. once #55080 is merged?
As suggested by @halleysfifthinc in #54855 (comment) --------- Co-authored-by: Max Horn <[email protected]>
@nanosoldier |
Why not a deprecation? |
I suppose I'm just more conservative here — to me this isn't worth an immediate minor change when the deprecation is so straightforward. It's great that you even already tackled the PkgEval failures, but I see that as rescuing two canaries from a particularly deep and obscure branch of our coal mine. I don't know who else is down there, and we might as well sound the alarm for anyone that's listening. |
Two of the original PkgEval failures are fixed by #55080. But you are right that there was one that needed to be fixed downstream. We can deprecate. |
This is still worth removing eventually because of #51710. I tagged 1.14 to be conservative, but would be open to landing it in 1.13, too. |
The package evaluation job you requested has completed - possible new issues were detected. |
Failure in |
I agree on both counts. That failure is fixed in emmt/InterProcessCommunication.jl#10. |
From triage: Keno and Oscar and Lilith are fine with removing this in 1.12 (and postponing the removal if anyone notices) |
As suggested by @halleysfifthinc in JuliaLang#54855 (comment) --------- Co-authored-by: Max Horn <[email protected]>
@nanosoldier |
The package evaluation job you requested has completed - possible new issues were detected. |
"Where did someone get a RawFD as an integer anyway?" -@StefanKarpinski
See also #51711
Fixes #51710