Skip to content

Better early return for add_launch_buttons if no launch buttons#693

Merged
choldgraf merged 1 commit intoexecutablebooks:masterfrom
paugier:disable-launch-buttons-alt
Feb 26, 2023
Merged

Better early return for add_launch_buttons if no launch buttons#693
choldgraf merged 1 commit intoexecutablebooks:masterfrom
paugier:disable-launch-buttons-alt

Conversation

@paugier
Copy link
Copy Markdown
Contributor

@paugier paugier commented Feb 25, 2023

@choldgraf
Copy link
Copy Markdown
Member

Nice - this looks correct to me. Could you just add a test to make sure that if none if them are there, then there is no launch buttons dropdo container on the page?

not launch_buttons
or not _is_notebook(app, pagename)
or not any(
launch_buttons[key]
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Shouldnt we use launch_buttons.get(key) in case one is not present so we don't raise a key error?

@choldgraf choldgraf merged commit 70e8ade into executablebooks:master Feb 26, 2023
@welcome
Copy link
Copy Markdown

welcome bot commented Feb 26, 2023

Congrats on your first merged pull request in this project! 🎉
congrats

Thank you for contributing, we are very proud of you! ❤️

@choldgraf
Copy link
Copy Markdown
Member

actually, no worries I will add a test in #691 - many thanks for this improvement :-)

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