Skip to content

Conversation

@daphn3k
Copy link

@daphn3k daphn3k commented Sep 13, 2025

fixes #1530

The actual fix is a couple of lines in sync.ipynb 's function _update_nb().
Below it, I have added two cells for comparing nbdev_update with and without the fix.
To be able to compare (as well as reproducing the bug) I had to add a new notebook (tests/update_directives.ipynb) and its exported py file (nbdev/test_update_directives.py).

This is my first pull request to this repo, so apologies if it doesn't fully conform. Feedback is welcome :)

@kurianbenoy-sarvam
Copy link

@daphn3k can you request Jeremy to review this PR?

@jph00
Copy link
Contributor

jph00 commented Oct 29, 2025

Many thanks for this PR! :)

tbh, it's way too much code. My basic premise is that I hate code. So every line of it must be justifiable by showing how much values it's adding. There are a lot of lines here not earning their keep. ;)

Instead, try to have just the absolute minimum about of new code to implement your feature, and to show it working. Don't add new files -- I add files of code even more than lines of code!

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.

nbdev_update duplicates directives

3 participants