fix isgenerated on nightly #86
Conversation
| @@ -2,6 +2,12 @@ using Core: CodeInfo, Typeof | |||
| using Core.Compiler: InferenceState, MethodInstance, svec | |||
| using InteractiveUtils: typesof | |||
|
|
|||
| if isdefined(Base, :hasgenerator) # VERSION >= v"1.7.0" | |||
There was a problem hiding this comment.
Use the version check already defined later instead, the symbols can exist together with different meanings
You also don't want to shadow base functions when the if condition later can satisfy the usage anyway.
There was a problem hiding this comment.
Note that the check like this will not work for dev versions of 1.7. You can use contrib/commit-name.sh in the Julia repo to get the exact version number for a given PR. That said, the solution here also seems fairly reasonable to me.
There was a problem hiding this comment.
I tried to run the commit-name.sh script but I get the following error (sorry for the italian)
carlo@prestige julia]$ contrib/commit-name.sh 1bb560ba1cf83eab8a4eb0315a60142a0dc95382
fatal: il percorso 'VERSION' esiste su disco, ma non in '1bb560ba1cf83eab8a4eb0315a60142a0dc95382'
contrib/commit-name.sh: riga 13: [: troppi argomenti
fatal: bad object 1bb560ba1cf83eab8a4eb0315a60142a0dc95382
fatal: bad object 1bb560ba1cf83eab8a4eb0315a60142a0dc95382
contrib/commit-name.sh: riga 27: [: =: atteso operatore unario
contrib/commit-name.sh: riga 30: [: =: atteso operatore unario
Can we keep the isdefined since it doesn't make much difference? Also, I don't see any overshadowing problem, hasgenerator is not exported from Base, and since we use it in a couple of spots I prefer to do the isdefined check once instead of multiple times
There was a problem hiding this comment.
You need to use the commit that was actually merged into master:
~/.../Julia/julia >>> ./contrib/commit-name.sh 2688a06
1.7.0-DEV.1096
But yeah, it's not really necessary here...
There was a problem hiding this comment.
You could use the images that are built by the julia build bots for the PR too.
I think the necessary version checks VERSION >. ... are already in place?
There was a problem hiding this comment.
I don't understand
There was a problem hiding this comment.
Julia's build bots build the images for every pr, so we could potentially use those is what I meant.
There was a problem hiding this comment.
Why not just use the nightly build? Or is that what you mean?
There was a problem hiding this comment.
can we stop discussing an issue that doesn't really exist and merge this? I'd like to have CI back on nightly
|
@DhairyaLGandhi merge and tag? |
|
@DhairyaLGandhi bump |
|
@DhairyaLGandhi can we merge this? |
|
See #88 |
|
see what? |
|
I got ci to trigger on nightly, so we at least have working tests |
|
needs a tag, then Zygote & co. tests will hopefully be passing again |
|
Doc test failures are real. |
isgeneratedwas renamed tohasgeneratorin JuliaLang/julia#40745