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

Only add extra_template_basedirs if it has not been set #1579

Merged
merged 1 commit into from
May 18, 2022

Conversation

tmetzl
Copy link
Contributor

@tmetzl tmetzl commented May 9, 2022

In nbgrader v0.6.x we only set the template_path option if it is not already in the config (see here).
In nbgrader v0.7.x we should also only set the the extra_template_basedirs conditionally in order to not break any custom exporters.

@jtpio jtpio added the bug label May 9, 2022
@jhamrick jhamrick changed the base branch from 0.7.x to master May 17, 2022 08:08
@jhamrick
Copy link
Member

Thanks @tmetzl !

Can you modify this to merge into master rather than 0.7.x? I think you just would need to cherry-pick your commit (so it excludes the 'Bump version to 0.7.1.dev' commit).

@jhamrick jhamrick added this to the 0.7.1 milestone May 17, 2022
@tmetzl
Copy link
Contributor Author

tmetzl commented May 17, 2022

@jhamrick I think I messed by dropping the commit. Should I create a new clean PR in favor of this one?

@jhamrick
Copy link
Member

I was able to fix it! 😄

(If you're curious, here are the git commands I used to do it)

# get the latest changes
git fetch upstream
# create a temp branch off of master to fix the PR
git checkout master
git branch patch-1-fix
# checkout the PR, then cherry-pick the relevent commit; this gives us a branch that only has the one commit in it, and not the others
gh pr checkout 1579
git checkout patch-1-fix
git cherry-pick 61df6a5
# reset the PR branch to be the same as the temp branch (throws away the other commits)
git checkout patch-1
git reset --hard patch-1-fix 
git push -f

@tmetzl
Copy link
Contributor Author

tmetzl commented May 17, 2022

Perfect 👍
Thank you for the git commands and the comments.

@jhamrick
Copy link
Member

Tests are passing locally, so should be good to merge.

@jhamrick jhamrick merged commit c9fff6e into jupyter:master May 18, 2022
@jhamrick
Copy link
Member

@meeseeksdev backport to 0.7.x

meeseeksmachine pushed a commit to meeseeksmachine/nbgrader that referenced this pull request May 18, 2022
jhamrick added a commit that referenced this pull request May 18, 2022
…9-on-0.7.x

Backport PR #1579 on branch 0.7.x (Only add extra_template_basedirs if it has not been set)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants