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

Added: Support for multiple templates per repository #1224

Merged

Conversation

RomHartmann
Copy link
Contributor

@RomHartmann RomHartmann commented Sep 4, 2019

For issue #1223

  • Implemented cli feature to find cookiecutter.json in a subdirectory. This was written to solve the problem that cookiecutter could not work with subdirectories within github. Also works with local, though that use case makes no sense :)
  • Updated unit tests for new cli option
  • Added a unit test to read from a subdir
  • Added documentation on how to use. Not sure where that should go though.
  • Manually tested it works with local path
  • Manually tested it works with local zip file
  • Manually tested it works with github

docs/advanced/repo_subdirectories.rst Outdated Show resolved Hide resolved
docs/advanced/repo_subdirectories.rst Outdated Show resolved Hide resolved
@insspb insspb added enhancement This issue/PR relates to a feature request. needs-review PR Only: This PR require review from other developer labels Sep 4, 2019
@RomHartmann
Copy link
Contributor Author

@insspb Should I also increment version in setup.py?

@insspb
Copy link
Member

insspb commented Sep 4, 2019

@insspb Should I also increment version in setup.py?

No, unfortunately, we have a long new version deploy process, so you should not increment it, as until several core developers review this it is hard to say when it will be merged. (Last PRs merged fast, but we have some dev depth with opened PRs since 2017)

@codecov
Copy link

codecov bot commented Sep 4, 2019

Codecov Report

Merging #1224 into master will decrease coverage by 0.12%.
The diff coverage is 80%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #1224      +/-   ##
==========================================
- Coverage     100%   99.87%   -0.13%     
==========================================
  Files          18       18              
  Lines         784      789       +5     
==========================================
+ Hits          784      788       +4     
- Misses          0        1       +1
Impacted Files Coverage Δ
cookiecutter/main.py 100% <ø> (ø) ⬆️
cookiecutter/cli.py 100% <100%> (ø) ⬆️
cookiecutter/repository.py 97.67% <75%> (-2.33%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 2ed56bf...e70e85a. Read the comment docs.

@codecov
Copy link

codecov bot commented Sep 4, 2019

Codecov Report

Merging #1224 into master will not change coverage.
The diff coverage is 100%.

Impacted file tree graph

@@          Coverage Diff           @@
##           master   #1224   +/-   ##
======================================
  Coverage     100%    100%           
======================================
  Files          18      18           
  Lines         784     788    +4     
======================================
+ Hits          784     788    +4
Impacted Files Coverage Δ
cookiecutter/main.py 100% <ø> (ø) ⬆️
cookiecutter/cli.py 100% <100%> (ø) ⬆️
cookiecutter/repository.py 100% <100%> (ø) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 2ed56bf...95185cd. Read the comment docs.

@RomHartmann
Copy link
Contributor Author

@insspb also added unit test so that coverage should now not go down anymore.

Please let me know if there is anything I can do to help speed up merging of PR. I really want to start using this feature (and I think another feature being reviewed right now #1173 😄 )

docs/advanced/index.rst Outdated Show resolved Hide resolved
docs/advanced/repo_subdirectories.rst Outdated Show resolved Hide resolved
@insspb
Copy link
Member

insspb commented Sep 5, 2019

@RomHartmann checked docs on local and get another errors after it.
Then I begin to fix it and catched that subdirectory is not correct name for this feature, as -cd flag too.
I made a PR RomHartmann#1 with all changes that will make it better I think.

@insspb insspb added this to the 1.7.0 milestone Sep 5, 2019
Copy link
Member

@insspb insspb left a comment

Choose a reason for hiding this comment

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

Wrong PR accidentally approved. Waiting for my PR merge.

@RomHartmann
Copy link
Contributor Author

@RomHartmann checked docs on local and get another errors after it.
Then I begin to fix it and catched that subdirectory is not correct name for this feature, as -cd flag too.
I made a PR RomHartmann#1 with all changes that will make it better I think.

@insspb Approved PR. Don't know what the merge policy is in this project, so I'll fall back to least-permission and wait for you to merge it :)

@insspb
Copy link
Member

insspb commented Sep 5, 2019

@RomHartmann you should merge my PR to your RomHartmann:1223-enable_repo_subdirectories repo. Then it will be available here automatically.
Core devs with write access can push code directly to your repo, but I do not have write access yet. So I am making PR.

Merge policy in cookiecutter:
Reviews and approve from 2-3 core at least, after one of cores marks PR as "Needs review"

@RomHartmann
Copy link
Contributor Author

@michaeljoseph Any other change suggestions?

Copy link
Contributor

@michaeljoseph michaeljoseph left a comment

Choose a reason for hiding this comment

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

👍👍👍

@michaeljoseph michaeljoseph added needs-merge and removed needs-review PR Only: This PR require review from other developer labels Nov 3, 2019
@SteveHNH
Copy link

Howdy folks! Interested in having this available for a project. Looks like it's all good to go but just needs to be merged?

@ryelo
Copy link

ryelo commented Dec 16, 2019

Seconding @SteveHNH, this would be great to use in our current project!

@ljvmiranda921
Copy link

ljvmiranda921 commented Dec 23, 2019

So excited for this feature to be merged!
Edit: Woops, I'm not sure why my approval appeared over there. Sorry about that!

1.8.0 Release automation moved this from In progress to Approved Dec 23, 2019
@insspb insspb added 1.7.1 and removed 1.8.0 labels Dec 23, 2019
@insspb insspb removed this from Approved in 1.8.0 Release Dec 23, 2019
@insspb insspb added this to In progress in 1.7.1 Release via automation Dec 23, 2019
@insspb insspb moved this from In progress to Reviewer approved in 1.7.1 Release Dec 23, 2019
@ssbarnea ssbarnea changed the title 1223 enable repo subdirectories Enable multiple templates per repository Jan 8, 2020
@ssbarnea ssbarnea merged commit b29ecbc into cookiecutter:master Jan 8, 2020
1.7.1 Release automation moved this from Reviewer approved to Done Jan 8, 2020
@insspb insspb added major Marks an important change, when major version update required and removed enhancement This issue/PR relates to a feature request. labels Apr 17, 2020
@insspb insspb changed the title Enable multiple templates per repository Added: multiple templates per repository support Apr 17, 2020
@ssbarnea ssbarnea changed the title Added: multiple templates per repository support Added support for multiple templates per repository Apr 17, 2020
@insspb insspb changed the title Added support for multiple templates per repository Added: Support for multiple templates per repository Apr 20, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
major Marks an important change, when major version update required
Projects
No open projects
1.7.1 Release
  
Done
Development

Successfully merging this pull request may close these issues.

None yet