-
-
Notifications
You must be signed in to change notification settings - Fork 300
refactor(Init): make project_info a module and remove self.project_info #1605
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
base: v4-9-2
Are you sure you want to change the base?
refactor(Init): make project_info a module and remove self.project_info #1605
Conversation
ac64612
to
d58b017
Compare
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## v4-9-2 #1605 +/- ##
=========================================
Coverage ? 98.86%
=========================================
Files ? 59
Lines ? 2654
Branches ? 0
=========================================
Hits ? 2624
Misses ? 30
Partials ? 0
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
d58b017
to
e22fcb3
Compare
Thanks for the quick review! |
name: str = questionary.select( | ||
"Please choose a supported config file: ", | ||
choices=CONFIG_FILES, | ||
default=default_path, |
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.
The suffix path
can sometimes refer to Path
objects. I consider it less confusing with the suffix filename
to indicate that it is a string.
tests/test_project_info.py
Outdated
class TestFileDetection: | ||
"""Test file detection functions.""" | ||
|
||
def test_has_pyproject_when_file_exists(self, chdir): |
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.
i think we can use parameterize for these
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.
Could you provide some details? I'm not sure how to make it more maintainable with parameterize.
Thanks 🙏
tests/test_project_info.py
Outdated
def test_has_uv_lock_when_file_exists(self, chdir): | ||
"""Test has_uv_lock returns True when uv.lock exists.""" | ||
Path("uv.lock").touch() | ||
assert project_info.has_uv_lock() is True |
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.
def test_has_uv_lock_when_file_exists(self, chdir): | |
"""Test has_uv_lock returns True when uv.lock exists.""" | |
Path("uv.lock").touch() | |
assert project_info.has_uv_lock() is True | |
@pytest.mark.parameterzie("file_name, func_name", [("uv.lock", "has_uv_lock"), ...]) | |
def test_return_true_when_file_exists(self, chdir, file_name: str, func_name: str): | |
"""Test has_uv_lock returns True when uv.lock exists.""" | |
Path(file_name).touch() | |
assert getattr(project_info, "has_uv_lock")() is True |
probably can try something like it
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.
Maybe it would be better if we just remove these simple has_XXX
functions.
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.
I think they're quite straightforward? Why do you think we should remove them
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.
Path(...).is_file()
is also straightforward.
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.
I will update a commit later. I am still testing locally.
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.
Now both project_info.py
and its test file are shorter.
e22fcb3
to
293b265
Compare
293b265
to
e03b1f0
Compare
Description
Make
project_info
a module asProjectInfo
only contains stateless helpers.