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.

Sign up for GitHub

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

Jump to bottom

run open in compare_examples.jl should be open_file #1143

Closed
ulysses4ever opened this issue May 4, 2018 · 5 comments
Closed

run open in compare_examples.jl should be open_file #1143

ulysses4ever opened this issue May 4, 2018 · 5 comments

Comments

@ulysses4ever
Copy link
Contributor

Regarding the issue about run open in test suit raised here. Gadfly.open_file is nice, but Gadly is huge and so fast to load. Hence the question, do we want to do using Gadfly in test suit (compare_examples.jl specifically)? Or maybe we simply copy-paste the definition of open_file inside it?
@bjarthur
Copy link
Member

bjarthur commented May 5, 2018

how about splitting open_file out into src/open_file.jl and then in test/compare_examples.jl doing an include(joinpath(@__DIR__,"..","src","open_file.jl")) ? a little kludgy, but better than duplicating code i think. thoughts? you'd need to add an include to Gadfly.jl as well.

@ulysses4ever
Copy link
Contributor Author

I like your idea. Will do.

ulysses4ever added a commit to ulysses4ever/Gadfly.jl that referenced this issue May 15, 2018
@ulysses4ever
Use Gadfly.open_file instead of run(open) in the test suite (fix Giov…
3a3d6c6 
…ineItalia#1143)
@ulysses4ever ulysses4ever mentioned this issue May 15, 2018
Merged
bjarthur added a commit that referenced this issue May 16, 2018
@bjarthur
Merge pull request #1146 from ulysses4ever/fix-regr-test-related
d31554b 
Use Gadfly.open_file instead of run(open) in the test suite (fix #1143)
bjarthur pushed a commit to bjarthur/Gadfly.jl that referenced this issue May 21, 2018
@ulysses4ever @bjarthur
Use Gadfly.open_file instead of run(open) in the test suite (fix Giov…
557e490 
…ineItalia#1143)
bjarthur pushed a commit to bjarthur/Gadfly.jl that referenced this issue May 22, 2018
@ulysses4ever @bjarthur
Use Gadfly.open_file instead of run(open) in the test suite (fix Giov…
41f29b4 
…ineItalia#1143)
@bjarthur
Copy link
Member

did you want to rename to devel-output and master-output as you suggested?

@ulysses4ever
Copy link
Contributor Author

ulysses4ever commented May 23, 2018 via email •
edited
Loading

bjarthur pushed a commit to bjarthur/Gadfly.jl that referenced this issue May 23, 2018
@ulysses4ever @bjarthur
Use Gadfly.open_file instead of run(open) in the test suite (fix Giov…
badd63d 
…ineItalia#1143)
bjarthur pushed a commit to bjarthur/Gadfly.jl that referenced this issue May 25, 2018
@ulysses4ever @bjarthur
Use Gadfly.open_file instead of run(open) in the test suite (fix Giov…
96f1980 
…ineItalia#1143)
@ulysses4ever
Copy link
Contributor Author

Here we go: #1154.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
2 participants
@bjarthur @ulysses4ever