-
-
Notifications
You must be signed in to change notification settings - Fork 2.7k
Be more forgiving when anything with opening a webbrowser fails #4161
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
Conversation
@jonmmease I think this is ready for review. Since you have written most of the file I touched, I'm tagging you, but feel free to nominate someone else of course.
I don't fully understand the failing release build, but I don't suspect I am causing it.
Update: I hovered over your name and it said "Former Chief Scientist at Plotly", so I'm not entirely sure whether you review pull requests still. Maybe let's also tag @chriddyp 😇
Hi @bartbroere, I'm not a maintainer of plotly projects any more, but FWIW these changes look reasonable to me!
@bartbroere the code change looks great, thanks! Just a couple of comments on the tests.
Co-authored-by: Alex Johnson <alex@plot.ly>
Thanks for the commit bringing it up to date. I might have some time tonight (GMT+2) to finish this PR and apply the feedback.
This reverts commit e2ec195.
@alexcjohnson I think it's ready for merge now
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.
💃 Thanks @bartbroere, looks great!
Uh oh!
There was an error while loading. Please reload this page.
This (hopefully) fixes an issue I encountered with Starboard Notebook. I'll link the issue here.