-
Notifications
You must be signed in to change notification settings - Fork 7
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
Add cruft compatibility, read from .cruft.json
#756
base: main
Are you sure you want to change the base?
Add cruft compatibility, read from .cruft.json
#756
Conversation
Currently failing Nox sessions:
|
This is still WIP. For some reason, `.cookiecutter.json` is still present.
This comment was marked as resolved.
This comment was marked as resolved.
I forgot to add the vanilla cookiecutter test back.
This seems to work correctly, and separate the tests for vanilla cookiecutter and cruft.
I believe this PR is now ready! |
It might be possible to remove some |
tests/conftest.py
Outdated
cookiecutter(template, no_input=True, output_dir=str(flavor_path)) | ||
return flavor_path / context["project_slug"] | ||
elif flavor == "cruft": | ||
import cruft # type: ignore[import] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
import cruft # type: ignore[import] | |
import cruft |
They merged it, now waiting for the release.
tests/conftest.py
Outdated
@pytest.fixture(params=["vanilla", "cruft"]) | ||
def flavor(request: pytest.FixtureRequest) -> str: | ||
"""Test flavor, either vanilla cookiecutter or cruft.""" | ||
out: str = request.param # type: ignore[attr-defined] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
out: str = request.param # type: ignore[attr-defined] | |
out: str = request.param |
This # type: ignore
can be removed by updating Pytest to >= 7.1 I believe.
I tried updating it with Poetry, but I believe it'd be better to update all dependencies at once.
@cjolowicz could you take a look at this PR? |
path = Path(".cookiecutter.json") | ||
text = repository.read_text(path, ref=ref) | ||
data = json.loads(text) | ||
except KeyError: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why KeyError?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is that what git.Repository.read_text
raises when it doesn't find the file?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It seems to be!
repository.read_text(path, ref=ref)
Traceback (most recent call last):
File "/home/micael/projects/retrocookie/src/retrocookie/core.py", line 42, in load_context
text = repository.read_text(path, ref=ref)
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
File "/home/micael/projects/retrocookie/src/retrocookie/git.py", line 200, in read_text
blob = functools.reduce(operator.truediv, path.parts, commit.tree)
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
KeyError: '.cookiecutter.json'
During handling of the above exception, another exception occurred:
Traceback (most recent call last):
File "<string>", line 1, in <module>
File "/home/micael/projects/retrocookie/src/retrocookie/git.py", line 200, in read_text
blob = functools.reduce(operator.truediv, path.parts, commit.tree)
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
KeyError: '.cookiecutter.json'
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do you think it'd be better to, for example, modify the read_text
method to raise a clearer error, like a FileNotFoundError
, instead?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Possibly, but not in this PR. Sorry for the slow review, have little bandwidth atm
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No worries. And sorry for pinging you. I had forgotten about this PR, then realized it had been over a year with no activity.
Closes #755