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

Minor/optional cleanup #3

Merged
merged 3 commits into from
May 23, 2024
Merged

Minor/optional cleanup #3

merged 3 commits into from
May 23, 2024

Conversation

jherland
Copy link
Member

Three minor things I found while reviewing this. Feel free to take zero or more of these commits. :-)

Commits:

  • Dockerfile: No need to activate virtualenv
  • Dockerfile: Use $VIRTUAL_ENV instead of duplicating its value
  • README.md: Minor tweaks

Invoking the 'pip' that is already installed in the virtualenv has the
same effect.
Insert a paragraph break before "Simple usage:".

Also, use "customized" instead of "extra" to describe the command line
options being passed in the advanced example. (With "extra" it reads
like the options are being appended to a list of existing - or default -
command line options, but in fact there are _no_ command line options
passed by default).
Copy link
Collaborator

@obscurerichard obscurerichard left a comment

Choose a reason for hiding this comment

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

👍🏻 :shipit:

This is more concise, I like it.

I'm probably going to be reworking this to eliminate the use of Docker to be more in line with what the isort and black plugins do, since FawltyDeps works best when run after you install the packages for the application under test, and in the same context. This is better than what was there though.

@obscurerichard obscurerichard merged commit 6b9e834 into main May 23, 2024
2 checks passed
@obscurerichard obscurerichard deleted the jherland/minor-cleanup branch May 23, 2024 17:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants