Skip to content
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

Merged
merged 3 commits into from
Feb 24, 2025
Merged

auto-generate contribute.md from notebook #1497

merged 3 commits into from
Feb 24, 2025

Conversation

bclavie
Copy link
Contributor

@bclavie bclavie commented Feb 21, 2025

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.

@bclavie bclavie requested a review from jph00 February 21, 2025 01:07
@jph00
Copy link
Contributor

jph00 commented Feb 21, 2025

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?

@bclavie
Copy link
Contributor Author

bclavie commented Feb 21, 2025

Hey, one of my main motivation was to actually have a good reason to dive into nbdev, the second one (correct me if I'm wrong @audreyfeldroy) is in the name of uniformisation so things are easier to automate. I've got no strong opinion against reverting so that this repo (or any repo that doesn't really leverage quarto/doesn't need the automation, really) doesn't need to use it and just uses plain ol' .md though!

@jph00
Copy link
Contributor

jph00 commented Feb 24, 2025

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.

Copy link
Contributor

@jph00 jph00 left a 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
):
"""
Copy link
Contributor

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
Copy link
Contributor

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):
Copy link
Contributor

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

Copy link
Contributor

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

@bclavie
Copy link
Contributor Author

bclavie commented Feb 24, 2025

Thanks! Updated to better match styling conventions.

@jph00 jph00 merged commit fd74e0f into main Feb 24, 2025
10 checks passed
@jph00 jph00 deleted the feat/contributing branch February 24, 2025 03:38
@jph00 jph00 added the enhancement New feature or request label Feb 24, 2025
@jph00
Copy link
Contributor

jph00 commented Feb 24, 2025

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.)

@jph00 jph00 changed the title feat: auto-generate contribute.md from nbs auto-generate contribute.md from notebook Feb 24, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants