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

feature: make install-all gather errors in batch #1348

Merged
merged 12 commits into from
Apr 20, 2024

Conversation

huxuan
Copy link
Member

@huxuan huxuan commented Apr 16, 2024

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

Summary of changes

As discussed in #1132 (comment), install-all needs a more robust strategy to deal with failed packages, so almost the same logic as reinstall-all is copied here. Besides, a test case for install-all is added.

Test plan

Tested by running

# command(s) to exercise these changes
nox -s tests

BTW, this pull request is a refactor, so no news is added and there is also no referenced issue. Let me know if it is necessary.

@huxuan huxuan changed the title Xuan.hu/more robust install all refactor: make install-all gather errors in batch Apr 16, 2024
@huxuan huxuan marked this pull request as ready for review April 16, 2024 08:24
@chrysle
Copy link
Contributor

chrysle commented Apr 16, 2024

Thanks for the quick submission; please add a changelog entry of type feature, as this is a fundamental change to the behaviour.

@huxuan
Copy link
Member Author

huxuan commented Apr 16, 2024

Thanks for the quick submission; please add a changelog entry of type feature, as this is a fundamental change to the behaviour.

Thanks for the response.

So what the issue number should be? Just use the pull request number or create a new issue for this?

I have use the pull request number directly as the issue number, let me know if I need to create a new issue for this.

@huxuan huxuan changed the title refactor: make install-all gather errors in batch feature: make install-all gather errors in batch Apr 16, 2024
@huxuan huxuan force-pushed the xuan.hu/more-robust-install-all branch from 2c8dac5 to eb9994b Compare April 17, 2024 01:00
tests/test_install_all.py Outdated Show resolved Hide resolved
@huxuan huxuan force-pushed the xuan.hu/more-robust-install-all branch from 6ccb727 to 8a5c027 Compare April 19, 2024 22:23
@huxuan huxuan requested a review from dukecat0 April 19, 2024 22:25
dukecat0
dukecat0 previously approved these changes Apr 20, 2024
Copy link
Member

@dukecat0 dukecat0 left a comment

Choose a reason for hiding this comment

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

Thanks!!

Co-authored-by: chrysle <fritzihab@posteo.de>
@huxuan huxuan requested a review from chrysle April 20, 2024 12:08
Copy link
Contributor

@chrysle chrysle left a comment

Choose a reason for hiding this comment

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

Looks good to me! Ha, and I was actually mistaken with the "stylistic convention" – but doesn't matter I guess.

@huxuan
Copy link
Member Author

huxuan commented Apr 20, 2024

Looks good to me! Ha, and I was actually mistaken with the "stylistic convention" – but doesn't matter I guess.

That is OK, we can follow up this in #1357. I will mark it as ready for review then.

@dukecat0 dukecat0 merged commit d8f4cab into pypa:main Apr 20, 2024
14 checks passed
@chrysle
Copy link
Contributor

chrysle commented Apr 20, 2024

Thanks!

@huxuan huxuan deleted the xuan.hu/more-robust-install-all branch April 20, 2024 15:25
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.

3 participants