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.

Sign up for GitHub

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

Jump to bottom

Enable --no-input option by adding docs and tests #8182

Merged
merged 3 commits into from
Jun 26, 2020
Merged

Enable --no-input option by adding docs and tests #8182

merged 3 commits into from
Jun 26, 2020

Conversation

deveshks
Copy link
Contributor

@deveshks deveshks commented May 1, 2020 •
edited
Loading

Fixes and closes #7688 , towards #2429

Added tests to verify the behaviour of authentication prompts, with and without the --no-input flag, and added help text to enable the option in docs and in --help

$ pip --help
.....
General Options:
.....
  --no-input                  Disable prompting for input.

@deveshks
Copy link
Contributor Author

deveshks commented May 6, 2020

May I get this PR reviewed?

@deveshks deveshks mentioned this pull request May 13, 2020
Open
@deveshks deveshks force-pushed the no-input-tests branch 3 times, most recently from b58f01c to a03c025 Compare May 16, 2020 04:47
pradyunsg
pradyunsg reviewed May 16, 2020
View reviewed changes
tests/functional/test_install_config.py Outdated
Comment on lines 248 to 249
print(result)
assert 'User for {}:{}'.format(server.host, server.port) in result.stdout
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
print(result)
assert 'User for {}:{}'.format(server.host, server.port) in result.stdout
assert 'User for {}:{}'.format(server.host, server.port) in result.stdout, str(result)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed in latest commit

pradyunsg
pradyunsg reviewed May 16, 2020
View reviewed changes
Copy link
Member

@pradyunsg pradyunsg left a comment

Choose a reason for hiding this comment

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

LGTM!

@deveshks deveshks requested a review from pradyunsg May 16, 2020 19:47
@deveshks
Copy link
Contributor Author

LGTM!

Thanks, I have addressed the review comments.

Could I also get the PR approved/merged as well?

@deveshks
Copy link
Contributor Author

deveshks commented Jun 1, 2020

Hi @pradyunsg ,

If there are no changes needed in this PR, I think this is ready to be approved/merged.

@deveshks
Copy link
Contributor Author

May I get this PR merged?

xavfernandez
xavfernandez reviewed Jun 25, 2020
View reviewed changes
news/7688.doc Outdated Show resolved Hide resolved
tests/functional/test_install_config.py Outdated Show resolved Hide resolved
@deveshks deveshks requested a review from xavfernandez June 26, 2020 01:58
@deveshks
Copy link
Contributor Author

Thanks for the approval @xavfernandez . May I please get this merged as well?

@xavfernandez xavfernandez merged commit 9a3c082 into pypa:master Jun 26, 2020
@xavfernandez
Copy link
Member

Thanks @deveshks 👍

@deveshks deveshks deleted the no-input-tests branch June 26, 2020 19:35
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Oct 12, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@xavfernandez xavfernandez xavfernandez approved these changes

@pradyunsg pradyunsg Awaiting requested review from pradyunsg

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Officially support --no-input command-line argument
3 participants
@deveshks @xavfernandez @pradyunsg