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

Change Zstd compression default to 19 #4663

Merged
merged 2 commits into from
Dec 2, 2022
Merged

Change Zstd compression default to 19 #4663

merged 2 commits into from
Dec 2, 2022

Conversation

jakirkham
Copy link
Member

@jakirkham jakirkham commented Dec 1, 2022 •
edited
Loading

Description

This uses less memory and takes less time while still providing a good level compression. Also lines up with how this was changed in conda-package-handling and conda-forge.

xref: conda/conda-package-handling#169
xref: conda/conda-package-handling#168
xref: conda-forge/conda-forge-ci-setup-feedstock#217

Checklist - did you ...

  • Add a file to the news directory (using the template) for the next release's release notes?
  • Add / update necessary tests?
  • Add / update outdated documentation?

@conda-bot conda-bot added the cla-signed [bot] added once the contributor has signed the CLA label Dec 1, 2022
@jakirkham jakirkham mentioned this pull request Dec 1, 2022
Merged
3 tasks
@jakirkham
Copy link
Member Author

cc @mbargull @dholth @jezdez

@mbargull
Copy link
Member

mbargull commented Dec 1, 2022

Could you also change/remove Defaults to the maximum. from the help text in conda_build.cli.main_build?

@jakirkham
Change Zstd compression default to 19
866c87e 
This uses less memory and takes less time while still providing a good
level compression. Also lines up with how this was changed in
conda-package-handling and conda-forge.
mbargull
mbargull approved these changes Dec 2, 2022
View reviewed changes
Copy link
Member

@mbargull mbargull left a comment

Choose a reason for hiding this comment

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

LGTM, thanks!

@jezdez jezdez requested a review from dholth December 2, 2022 13:45
dholth
dholth approved these changes Dec 2, 2022
View reviewed changes
Copy link
Contributor

@dholth dholth left a comment

Choose a reason for hiding this comment

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

Lgtm

@mbargull
Copy link
Member

mbargull commented Dec 2, 2022 •
edited
Loading

One, two, three, merge :).
(But I don't seem to have merge access anymore :D.)

@dholth dholth merged commit 53aa0d8 into conda:main Dec 2, 2022
@jakirkham jakirkham deleted the zst_def_19 branch December 2, 2022 17:28
@jakirkham
Copy link
Member Author

Thanks all! 🙏

@github-actions github-actions bot added the locked [bot] locked due to inactivity label Dec 3, 2023
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Dec 3, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@mbargull mbargull mbargull approved these changes

@dholth dholth dholth approved these changes

@jezdez jezdez jezdez approved these changes

Assignees
No one assigned
Labels
cla-signed [bot] added once the contributor has signed the CLA locked [bot] locked due to inactivity
Projects
🧭 Planning
Archived in project
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@jakirkham @mbargull @jezdez @dholth @conda-bot