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

Remove "--always-unzip" option #8470

Merged
merged 4 commits into from
Jun 30, 2020
Merged

Remove "--always-unzip" option #8470

merged 4 commits into from
Jun 30, 2020

Conversation

deveshks
Copy link
Contributor

Closes #8408

This was referenced Jun 19, 2020
Closed
Open
@deveshks
Copy link
Contributor Author

Hi @pfmoore,

Given that you commented on #8408 (comment) to get this PR rolling, I would love to get your eyes on this, as to whether this is the right way to remove the option.

@pradyunsg
Copy link
Member

I suggest flipping the order of removals, to first remove the tests, then the corresponding code and then the flag. This will make it easier to keep our history functional during git bisect. :)

@deveshks deveshks force-pushed the remove-always-unzip branch 4 times, most recently from 5b33561 to 53de840 Compare June 24, 2020 16:16
@deveshks
Copy link
Contributor Author

deveshks commented Jun 24, 2020 •
edited
Loading

I suggest flipping the order of removals, to first remove the tests, then the corresponding code and then the flag. This will make it easier to keep our history functional during git bisect. :)

That's a good idea @pradyunsg . I have ordered the git commits accordingly. Please have a look.

(I also see that some of the yaml based tests are failing in CI in master.)

@deveshks deveshks force-pushed the remove-always-unzip branch from 53de840 to 7970eb7 Compare June 25, 2020 11:22
@deveshks
Copy link
Contributor Author

The CI is green now and this should be ready for further reviews.

@xavfernandez
Copy link
Member

Even if the option was not documented, I think it should still have a proper removal news entry, something likeRemove undocumented and deprecated option --always-unzip ?

@deveshks deveshks force-pushed the remove-always-unzip branch from 7970eb7 to 504759b Compare June 26, 2020 19:37
@deveshks
Copy link
Contributor Author

deveshks commented Jun 26, 2020 •
edited
Loading

Even if the option was not documented, I think it should still have a proper removal news entry, something likeRemove undocumented and deprecated option --always-unzip ?

Oops, my bad @xavfernandez . I have added the suggested wording to the news entry in 504759b . Please take a look.

@deveshks
Copy link
Contributor Author

Thanks for the approval. The CI is green now and this can be merged :)

@pradyunsg pradyunsg merged commit 93b0683 into pypa:master Jun 30, 2020
@deveshks deveshks deleted the remove-always-unzip branch June 30, 2020 13:37
@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

@pradyunsg pradyunsg pradyunsg approved these changes

@xavfernandez xavfernandez xavfernandez approved these changes

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

Successfully merging this pull request may close these issues.

Remove --always-unzip
3 participants
@deveshks @pradyunsg @xavfernandez