CASSGO-33 snappy has been moved to a separate package #1844
Conversation
|
@OleksiienkoMykyta can you rebase this branch? Or even start from scratch (from trunk) and force push to this branch, might be easier |
Sure, I'll handle this on the next week |
OleksiienkoMykyta
force-pushed
the
CASSGO-33-snappy-has-been-moved-to-a-separate-package
branch
2 times, most recently
from
d373fb6 to
0f254c1
Compare
|
@joao-r-reis, I have fixed the conflicts. |
CHANGELOG.md
Outdated
| @@ -18,6 +18,9 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 | |||
|
|
|||
| ### Changed | |||
|
|
|||
| - Moved the Snappy compressor into its own separate package. | |||
| This allows users to include only the compression dependencies they need. (CASSGO-33) | |||
There was a problem hiding this comment.
I think we don't need this last line, it would be good to keep this list brief, users can open the linked JIRAs if they want more details.
There was a problem hiding this comment.
I agree, fixed.
joao-r-reis
changed the title
There was a problem hiding this comment.
Left 1 comment about the change in CHANGELOG.md and the new file name for the snappy compressor.
I'm not sure if the tests under snappy/compressor_test.go will run. I believe this is being handled in #1864 but it would be nice to get confirmation that these tests are passing before merging this.
Maybe we can wait for #1864 to be merged before merging this one.
snappy/snappy-compressor.go
Outdated
| @@ -0,0 +1,28 @@ | |||
| package snappy | |||
|
|
|||
| import "github.com/golang/snappy" | |||
There was a problem hiding this comment.
We can have this file be named compressor.go no?
There was a problem hiding this comment.
I tried to align it with lz4, but sure, we can keep it just as a compressor.go.
Fixed.
joao-r-reis
changed the title
OleksiienkoMykyta
force-pushed
the
CASSGO-33-snappy-has-been-moved-to-a-separate-package
branch
from
0f254c1 to
e6fd92d
Compare
Sure, let's wait for it. It's always better to double-check |
|
Can you rebase this branch so it gets the changes from trunk? With this I think we will see the snappy unit tests run on CI |
Currently, Snappy is downloaded and included even when users only require LZ4 compression. To streamline the driver and reduce unnecessary dependency overhead for users who don't utilize Snappy, move the Snappy compressor into its own separate package. This will allow users to include only the compression dependencies they need. Patch by Mykyta Oleksiienko; reviewed by Joao Reis CASSGO-33
OleksiienkoMykyta
force-pushed
the
CASSGO-33-snappy-has-been-moved-to-a-separate-package
branch
from
e6fd92d to
20ff614
Compare
| @@ -22,7 +22,7 @@ | |||
| * See the NOTICE file distributed with this work for additional information. | |||
| */ | |||
|
|
|||
| package gocql | |||
| package snappy | |||
There was a problem hiding this comment.
This file is missing
//go:build all || unit
// +build all unit
so it runs in the CI unit tests step
|
integration tests with snappy appear fine so when you add this I'll merge this pr |
|
@OleksiienkoMykyta do you have time to address this? We're trying to resolve the remaining pending PRs for the 2.0 release |
This PR should be reviewed right after #1822 is merged.