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

fix #1091: nbdev_new pins on major+minor version of nbdev-template #1094

Closed
wants to merge 1 commit into from

Conversation

seeM
Copy link
Contributor

@seeM seeM commented Sep 18, 2022

@seeM seeM requested review from jph00 and hamelsmu September 18, 2022 23:34
@review-notebook-app
Copy link

Check out this pull request on  ReviewNB

See visual diffs & provide feedback on Jupyter Notebooks.


Powered by ReviewNB

@@ -172,6 +172,25 @@
" \"Use a token with the correction permissions or perform these steps manually.\")"
Copy link
Contributor

Choose a reason for hiding this comment

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

Line #13.            tag = _nbdev_template_release((1,1)).tag_name

I wonder if there is a way to make it such that we can remind ourselves that we must change this line of code when we make a breaking change in the template repository. Surely, we will forget to this otherwise?

Any ideas?


Reply via ReviewNB

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good point. I don't have a good answer yet, best I can think of is an e2e test for nbdev_new that (hopefully) fails if we forget to bump

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@jph00 I'm curious to hear your take on this

@seeM
Copy link
Contributor Author

seeM commented Sep 21, 2022

Closing. #1103 is a better solution to the underlying problem

@seeM seeM closed this Sep 21, 2022
@seeM seeM deleted the fix-1091 branch September 21, 2022 01:48
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_new pins on major+minor version of nbdev-template
2 participants