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

bug: Removed --python flag dependency when --force is passed #1306

Merged
merged 24 commits into from
Mar 31, 2024

Conversation

SwarajBaral
Copy link
Contributor

Fixes #1304

  • I have added a news fragment under changelog.d/ (if the patch affects the end users)

Summary of changes

Test plan

Tested by running

# command(s) to exercise these changes

Copy link
Contributor

@Gitznik Gitznik left a comment

Choose a reason for hiding this comment

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

Thanks for having a look. I'm not convinces this actually adresses the issue though. Please see my other review comment.

src/pipx/commands/install.py Outdated Show resolved Hide resolved
src/pipx/main.py Outdated Show resolved Hide resolved
src/pipx/commands/install.py Outdated Show resolved Hide resolved
src/pipx/main.py Outdated Show resolved Hide resolved
src/pipx/commands/install.py Outdated Show resolved Hide resolved
Copy link
Contributor

@Gitznik Gitznik left a comment

Choose a reason for hiding this comment

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

Code changes look fine to me. Can you please add a test that verifies this behavior?

@SwarajBaral
Copy link
Contributor Author

Code changes look fine to me. Can you please add a test that verifies this behavior?

@Gitznik Doesn't test_passed_python_and_force_flag_warning test this exact behaviour ? I am happy to write a test for this, just not sure what I should be testing here that is not covered already.

@Gitznik
Copy link
Contributor

Gitznik commented Mar 30, 2024

@Gitznik Doesn't test_passed_python_and_force_flag_warning test this exact behaviour ? I am happy to write a test for this, just not sure what I should be testing here that is not covered already.

That test didn't cover the bug you fixed, so we need a test that verifies that this bug can no longer happen.

@SwarajBaral
Copy link
Contributor Author

That test didn't cover the bug you fixed, so we need a test that verifies that this bug can no longer happen.

Done. Also added changelog.md file

tests/test_install.py Outdated Show resolved Hide resolved
changelog.d/1304.bugfix.md Outdated Show resolved Hide resolved
@Gitznik Gitznik merged commit f8e7bfc into pypa:main Mar 31, 2024
14 checks passed
Koppom94

This comment was marked as spam.

@pypa pypa deleted a comment from Koppom94 Jul 31, 2024
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.

pipx install --force without --python complains about the ignorance of --python
4 participants