-
Notifications
You must be signed in to change notification settings - Fork 511
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
auto-generate contribute.md from notebook #1497
Conversation
Thanks @bclavie :) I'm happy to merge this PR, although I don't understand the point of it. Why use ipynb for this? If we don't have a good reason to suggest it, perhaps we should leave nbdev's as a .md? |
Hey, one of my main motivation was to actually have a good reason to dive into |
Yeah let's keep the feature, but revert the use of it for the .md file here. AFAIK there isn't a benefit to auto-generating it, unless you need notebook features in it for some reason. |
1d70d06
to
f90a71d
Compare
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 @bclavie :D
Minor style requests to look at before we merge. (I haven't mentioned all of them - just an example of each)
nbdev/quarto.py
Outdated
path:str=None, # Path to notebooks (defaults to nbs_path) | ||
chk_time:bool=False # Only build if out-of-date | ||
): | ||
""" |
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.
docstrings should be on a single line.
nbdev/quarto.py
Outdated
if chk_time and _doc_mtime_not_older(contrib_md, contrib_nb_path): | ||
return | ||
|
||
# If there's no contributing notebook, do nothing |
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.
comments should not just repeat what the code already does - only add comments where there's something surprising or hard to understand
nbdev/quarto.py
Outdated
contrib_md = cfg.config_path / 'CONTRIBUTING.md' | ||
|
||
# If out of date check is requested, skip if up-to-date or missing | ||
if chk_time and _doc_mtime_not_older(contrib_md, contrib_nb_path): |
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.
single ideas should be on a single line (e.g in this case the if
and the body)
# Decide which notebook is your "contributing" NB (you can hardcode or add to settings.ini) | ||
contrib_nb_name = cfg.get('contributing_nb', 'contributing.ipynb') | ||
contrib_nb_path = path / contrib_nb_name | ||
|
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.
empty lines should only be added where needed to separate substantive sections
Thanks! Updated to better match styling conventions. |
Super. BTW for generating release notes, we use "enhancement" and "bug" labels, rather than adding a string prefix to the PR title. (I've added that now.) |
This reproduces the same behaviour as for README.md, but for CONTRIBUTING.md, so every important root-level markdown file can be quarto-generated from notebooks.
Also puts our money where our mouth is, and replaces the manual
CONTRIBUTING.md
with the auto-generated one.