-
-
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
Revert "Enable JuliaSyntax.jl as the default Julia parser" #50227
Conversation
cc: @oscardssmith @c42f |
See #50205. It appears to be unrelated. |
I think this PR pushed it through the brink, but it has been broken on the cusp of breaking for a while. |
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.
I don't think having CI systematically broken is acceptable. As Gabriel said, it has been close to breaking for a while, but now CI is finally red everywhere.
The underlying problem has to be addressed as quickly as possible, but it shouldn't be used as excuse for having useless CI.
I don't think we should revert this wholesale. Better to add a build-time option to not have JuilaSyntax as the default parser in the sysimage? |
It'll take a little investigation to add a build-time option but hopefully I can have a PR for that in fairly short order. |
I think build problems have been mitigated by JuliaCI/julia-buildkite#308? |
Ok excellent. So I don't need to rush forward making a quick fix at this point? |
I don't think so. Also, it was a more fundamental problem than avoiding building JuliaSyntax, another large change would have pushed beyond the limit anyway, so an ad-hoc solution for a single PR wouldn't be sufficient. I think also #50237 is meant to further reduce memory usage in general. |
Not sure if it is the same as for 32bit windows, but I can no longer build the mac app from source (contrib/mac/app), apparently due to a JuliaSyntax issue. It fails with the following error
|
No. You want to open a new issue for that. |
Done, #50245 |
Reverts #46372
It looks like that PR broke the
build
job for 32-bit Windows.