Skip to content

Conversation

@bilke
Copy link

@bilke bilke commented Dec 21, 2021

To be used by CI as build artifacts.

Initial implementation by @chleh.

Copy link
Member

@amit1rrr amit1rrr left a comment

Choose a reason for hiding this comment

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

Thank you for the contribution. --html flag looks like a good addition to treon. I've left some comments.

P.S. - I'll not be able to review again for next 8-10 days as I'm out of office. But will look out for this in early January. Happy Holidays.

try:
processor.preprocess(notebook, metadata(path))
except CellExecutionError:
if html:
Copy link
Member

Choose a reason for hiding this comment

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

It seems like html will only be generated if there's execution error. If there's an HTML flag then I'm guessing user is expecting HTML output even if the notebook executed successfully.

setup_logging(arguments)
LOG.info('Executing treon version %s', __version__)
thread_count = arguments['--threads'] or DEFAULT_THREAD_COUNT
html_output = arguments['--html'] or False
Copy link
Member

Choose a reason for hiding this comment

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

Maybe convert to bool i.e. bool(arguments['--html']) so that we're sure it's a boolean.

Let's use is_html as the variable name to indicate it's a boolean.

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.

2 participants