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

stop extending nonexistent Base.quit #190

Merged
merged 1 commit into from
Mar 26, 2019
Merged

stop extending nonexistent Base.quit #190

merged 1 commit into from
Mar 26, 2019

Conversation

pfitzseb
Copy link
Member

@pfitzseb pfitzseb commented Mar 7, 2019

Fixes #142.

@NHDaly NHDaly closed this Mar 13, 2019
@NHDaly NHDaly reopened this Mar 13, 2019
@NHDaly
Copy link
Collaborator

NHDaly commented Mar 13, 2019

Reopened PR to restart tests (i think there was a travis issue?)

Copy link
Collaborator

@NHDaly NHDaly left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM assuming tests pass tho

@pfitzseb pfitzseb closed this Mar 13, 2019
@pfitzseb pfitzseb reopened this Mar 13, 2019
@NHDaly
Copy link
Collaborator

NHDaly commented Mar 13, 2019

Huh.. the test failure is consistent.

It seems to be a deadlock or infinite loop?

@pfitzseb
Copy link
Member Author

pfitzseb commented Mar 13, 2019

I'm getting
image
locally when testing Blink even without this PR. Not sure what's going on there.

@twavv
Copy link
Member

twavv commented Mar 25, 2019

CI for this should be re-run after #193.

@pfitzseb pfitzseb closed this Mar 25, 2019
@pfitzseb pfitzseb reopened this Mar 25, 2019
@codecov-io
Copy link

Codecov Report

Merging #190 into master will decrease coverage by 8.88%.
The diff coverage is 0%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #190      +/-   ##
==========================================
- Coverage   44.87%   35.98%   -8.89%     
==========================================
  Files          11        9       -2     
  Lines         312      314       +2     
==========================================
- Hits          140      113      -27     
- Misses        172      201      +29
Impacted Files Coverage Δ
src/AtomShell/process.jl 58.33% <0%> (-17.28%) ⬇️
src/content/content.jl 52.38% <0%> (-28.58%) ⬇️
src/rpc/callbacks.jl 76.92% <0%> (-15.39%) ⬇️
src/content/server.jl 61.9% <0%> (-14.29%) ⬇️
src/rpc/rpc.jl 64.7% <0%> (-7.52%) ⬇️
src/AtomShell/window.jl 26.76% <0%> (-7.49%) ⬇️
src/AtomShell/AtomShell.jl
src/Blink.jl

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 93edf72...287e39a. Read the comment docs.

@pfitzseb pfitzseb merged commit 9fa930a into master Mar 26, 2019
@pfitzseb pfitzseb deleted the sp/quit branch March 26, 2019 08:42
@comaldave
Copy link

I am still seeing this error.

  Building Blink → `~/.juliapro/JuliaPro_v1.1.1.1/packages/Blink/1QOOi/deps/build.log`

julia> using Blink
[ Info: Precompiling Blink [ad839575-38b3-5650-b840-f874b8c74a25]
WARNING: could not import Base.quit into AtomShell

Seems safe to ignore. I would like to help remove the error message. I have been using Julia for 2 hours, very nice.

@NHDaly
Copy link
Collaborator

NHDaly commented Jun 20, 2019

Thanks for the error report! 😁 Welcome to julia; I'm glad you're having a nice experience so far!

Yeah, this was fixed by this PR on the master branch of this repository, but that change hasn't been released yet, so you're still seeing an older version. We're working on releasing it right now! (the release process has changed since our last release, so it's taking a bit longer than usual.)

Thanks for filing an error report. I hope you keep enjoying using Julia! :)

More info here:
#189 (comment)

@haydenfree
Copy link

@NHDaly Is there any update on when the new release will be tagged? Thanks so much!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

What to do about quit(::ElectronShell)?
6 participants