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

[PM-3666] snap: set compression to lzo #6132

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

pachulo
Copy link
Contributor

@pachulo pachulo commented Aug 28, 2023

Type of change

- [ ] Bug fix
- [ ] New feature development
- [x] Tech debt (refactoring, code cleanup, dependency upgrades, etc)
- [ ] Build/deploy pipeline (DevOps)
- [ ] Other

Objective

With #5038 being now merged, you should build the snap with the compression algorithm that gives the best results for desktop apps; according to Canonical, the use of the LZO compression offers 40-74% cold startup improvements over the XZ compression:

Code changes

I've just added the compression option in the top snap key, and configured the value to lzo

Should fix #2609

@pachulo pachulo requested a review from a team as a code owner August 28, 2023 21:52
@pachulo pachulo changed the title snap: set lzo compression snap: set compression to lzo Aug 28, 2023
@bitwarden-bot
Copy link

Thank you for your contribution! We've added this to our internal Community PR board for review.
ID: PM-3666

@bitwarden-bot bitwarden-bot changed the title snap: set compression to lzo [PM-3666] snap: set compression to lzo Aug 28, 2023
@withinfocus withinfocus requested a review from a team August 29, 2023 12:48
@ajkavanagh
Copy link

Bump; come on let's get this merged. The snap is very slow still.

@djsmith85 djsmith85 self-assigned this Jan 11, 2024
Copy link
Contributor

@djsmith85 djsmith85 left a comment

Choose a reason for hiding this comment

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

@pachulo Thank you for your contribution and apologies on the long wait for a review.

With #7448 we also bumped the snap-core version from core18 to core22, which we expect to bring some more improvements for the Bitwarden snap.

Approving the changes. Going to create a separate branch to kick-off all the build workflows, which will gives us feedback on new file-sizes. Then passing it on to QA for testing.

@djsmith85
Copy link
Contributor

The build is currently failing https://github.com/bitwarden/clients/actions/runs/7490825186/job/20390598188?pr=7517

Didn't quite get why AppImage is complaining, because we are changing the compression for snap. We'll need to look into it. Seems this issue has been reported at electron-userland/electron-builder#7013

@djsmith85 djsmith85 added the needs-qa Marks a PR as requiring QA approval label Jan 11, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
community-pr needs-qa Marks a PR as requiring QA approval
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Enable LZO compression for snap package
5 participants