Skip to content

Add the possibility to disable launch buttons from Jupyter Book - #692

Closed
paugier wants to merge 1 commit into
executablebooks:masterfrom
paugier:disable-launch-buttons
Closed

Add the possibility to disable launch buttons from Jupyter Book#692
paugier wants to merge 1 commit into
executablebooks:masterfrom
paugier:disable-launch-buttons

Conversation

@paugier

@paugier paugier commented Feb 24, 2023

Copy link
Copy Markdown
Contributor

@welcome

welcome Bot commented Feb 24, 2023

Copy link
Copy Markdown

Thanks for submitting your first pull request! You are awesome! 🤗

If you haven't done so already, check out EBP's Code of Conduct and our Contributing Guide, as this will greatly help the review process.

Welcome to the EBP community! 🎉

@choldgraf

choldgraf commented Feb 24, 2023

Copy link
Copy Markdown
Member

This seems like a reasonable fix to me, but I wanted to throw out one other idea because it might be cleaner:

The goal is for launch buttons only to show up if a user has configured them to do so. I think the real problem is these things:

  • Jupyter book sets "mybinder.org" as the default binder instance.
  • The check for whether the buttons should exist only checks for the presence of any launch button config, as opposed to specifically checking whether a launch button service has been requested (eg binder, colab, etc)

So a way to fix this without changing config values would be to make launch buttons only show if at least one service is explicitly specified, rather than defaulting to mybinder.org. it would mean that users explicitly have to put mybinder.org, but maybe that's ok?

So i guess the decision point is: is it better to ask people to manually disable launch buttons if they don't want them, or better to manually enable mybinder.org rather than default to it

What do you think?

@paugier

paugier commented Feb 24, 2023

Copy link
Copy Markdown
Contributor Author

I don't have strong opinion on what is better ("manually disable" or "manually enable").

An advantage of the solution "manually disable" is that it is not a breaking change. Most users won't be affected by this change.

@choldgraf

Copy link
Copy Markdown
Member

Now that I think about it more, I think we want the default in JB to be "no launch buttons unless you explicitly define a service". That's because otherwise there is no guarantee that the repository is actually buildable on mybinder.org . So we don't want to show a button to use binder if we don't know binder is even an option

@paugier

paugier commented Feb 24, 2023

Copy link
Copy Markdown
Contributor Author

Yes, it makes sense. Breaking change but for a good reason.

For people working with something else than Github it's even more convenient because the warnings will disappear without doing anything 🙂

So if I understand correctly I guess this PR can be closed and replaced by a new PR that could also address #462.

@choldgraf

Copy link
Copy Markdown
Member

closing this since it was superceded by #693 and the change to jupyter book

@choldgraf choldgraf closed this Feb 27, 2023
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.

Silence WARNING: Currently Binder/JupyterHub repositories must be on GitHub, got https://...

2 participants