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

Make brotli and zstd optional again #723

Merged
merged 3 commits into from
Apr 9, 2024
Merged

Make brotli and zstd optional again #723

merged 3 commits into from
Apr 9, 2024

Conversation

DerGuteMoritz
Copy link
Collaborator

Since it came up a few times now, I looked into making zstd and brotli optional again. Turns out there was a pretty easy workaround (see second commit). With this branch, the repro from #703 doesn't crash anymore.

@DerGuteMoritz DerGuteMoritz mentioned this pull request Apr 6, 2024
Merged
KingMob
KingMob reviewed Apr 7, 2024
View reviewed changes
Copy link
Collaborator

@KingMob KingMob left a comment

Choose a reason for hiding this comment

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

I wonder if we're just shifting around the set of people who will complain :)

Though truthfully, based on the number of downloads I see on Clojars, I don't think Aleph is used much anymore, so there probably won't be many complaints.

@DerGuteMoritz
Copy link
Collaborator Author

I wonder if we're just shifting around the set of people who will complain :)

True. I guess we could have another vote about it but given the meager turnout last time...

Though truthfully, based on the number of downloads I see on Clojars, I don't think Aleph is used much anymore, so there probably won't be many complaints.

Hmm indeed.... judging from these, at best there are maybe still 1000 users around? Perhaps it's time for a bit of a marketing campaign to re-ignite its popularity 😄

@DerGuteMoritz
Re-implement compression options check in Java
5e46907 
The Clojure implementation needed a hacky workaround[1] which felt too fragile after all.

[1] Due to https://clojure.atlassian.net/browse/CLJ-2842
KingMob
KingMob approved these changes Apr 9, 2024
View reviewed changes
Copy link
Collaborator

@KingMob KingMob 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. I think it's much safer to move this to Java.

@DerGuteMoritz DerGuteMoritz merged commit f887290 into master Apr 9, 2024
1 check passed
@DerGuteMoritz DerGuteMoritz deleted the make-brotli-and-zstd-optional-again branch April 9, 2024 14:00
@bitti bitti mentioned this pull request May 22, 2024
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@arnaudgeiser arnaudgeiser arnaudgeiser approved these changes

@KingMob KingMob KingMob 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.
3 participants
@DerGuteMoritz @KingMob @arnaudgeiser