-
Notifications
You must be signed in to change notification settings - Fork 20
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
Automatically use requirements.in
if the project uses a requirements.txt
+ requirements.in
-setup
#641
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #641 +/- ##
=====================================
Coverage 92.4% 92.4%
=====================================
Files 35 35
Lines 950 958 +8
Branches 175 176 +1
=====================================
+ Hits 878 886 +8
Misses 55 55
Partials 17 17 ☔ View full report in Codecov by Sentry. |
Reviewing this made me think that we could simplify a bit the dependency specification detector by getting rid of the intermediate Aside from this refactoring, I'm wondering about 2 things:
|
Yes, definitely! In the earlier approach I did not do that because of the way we had the
I think we should not do that, for two reasons:
|
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.
Nice work 🙂
PR Checklist
docs
is updatedDescription of changes
This implements a solution that was proposed by @mkniewallner for the issue described in #608. That is; use a same approach as we use for the
exclude
parameter to determine if a user specified the argumentrequirement-files
. If no input is provided by the user for this argument and the project contains both a requirements.txt and a requirements.in, use the requirements.in.Considerations:
requirements.in
file?DependencySpecificationDetector().detect()
: This would make sure thatdeptry
also works if the project only contains arequirements.in
and norequirements.txt
. But this will log to the user that therequirements.in
will be used before theDependencySpecificationDetector
has run. This will be a lie if the project detects that Poetry is used for dependency management in theDependencySpecificationDetector
.DependencySpecificationDetector
-> this seems to violate SRP; now it not only detects, but also overwrites the requirements_files argument if it decides that is necessary.DependencySpecificationDetector().detect()
: This means it does not work automatically if the project only contains arequirements.in
and norequirements.txt
. This should be a rare case, so this solution seems like the best solution to me.If the user does not explicitly set the
requirements-files
argument, the dependency format is detected asrequirements-file
because the project contains arequirements.txt
, andrequirements.in
is detected, the user will see the following message: