Skip to content
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

fix #27546 by using jl_set_global instead of eval to set Main.ans #27562

Merged
merged 2 commits into from
Jun 18, 2018

Conversation

jrevels
Copy link
Member

@jrevels jrevels commented Jun 13, 2018

Thanks to @JeffBezanson for pointing out the problem/this workaround.

Still needs a test.

@StefanKarpinski StefanKarpinski added needs tests Unit tests are required for this change stdlib:REPL Julia's REPL (Read Eval Print Loop) labels Jun 13, 2018
@jrevels
Copy link
Member Author

jrevels commented Jun 13, 2018

I added a test, but I don't really like it - seems a bit fragile and dependent on somewhat irrelevant things (like how we pretty-print Exprs).

If I was less noobish with the REPL code, I'd try to actually test the resultant object/check that no errors are generated without depending on checking pretty-printed results.

@jrevels jrevels removed the needs tests Unit tests are required for this change label Jun 13, 2018
@jrevels
Copy link
Member Author

jrevels commented Jun 18, 2018

CircleCI failure is unrelated (it's the Could not locate challenge: "Private key location for '[email protected]':"... thing that's been popping up in other PRs as well).

I'm also pretty sure the Travis macOS failure is unrelated, unless our FileWatching for some reason relied on the REPL 😛Error is:

FileWatching: Test Failed at /private/tmp/julia/share/julia/stdlib/v0.7/FileWatching/test/runtests.jl:401
  Expression: pop!(changes) == (F_PATH => FileWatching.FileEvent(FileWatching.UV_RENAME))
   Evaluated: "afile.txt3" => FileWatching.FileEvent(true, false, false) == "afile.txt" => FileWatching.FileEvent(true, false, false)

With a happy green checkmark from Jeff, and CI mostly green, I'll go ahead and merge.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
stdlib:REPL Julia's REPL (Read Eval Print Loop)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants