CASSGO-32 move lz4 compressor to lz4 package within the gocql module #1842
Conversation
|
I assume you want this to be looked at after the v5 PR is merged? Since it contains all of those commits. |
Yes, I am sorry that I didn't add the description earlier, I think it should be in WIP until the v5 PR is merged |
OleksiienkoMykyta
changed the title
Hmm I think it would be better to do the opposite so that we can make the v5 PR add proper testing with lz4 without the "hack", so if you can cherry pick your commit on top of |
I agree with @joao-r-reis. If this PR gets merged before #1822 it will allow me to eliminate the go.mod |
|
@joao-r-reis @worryg0d, I agree too, it will be much easier this way. I'll do it |
OleksiienkoMykyta
force-pushed
the
CASSGO-32-lz4-has-been-moved-into-gocql-package
branch
from
1a3487e
to
efe9a1b
Compare
CHANGELOG.md
Outdated
| @@ -10,6 +10,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 | |||
|
|
|||
| ### Changed | |||
|
|
|||
| - CASSGO-32 Move lz4 Compressor into gocql package | |||
There was a problem hiding this comment.
follow the format of the other lines
go.mod
Outdated
| gopkg.in/yaml.v3 v3.0.1 // indirect | ||
| ) | ||
|
|
||
| go 1.19 |
There was a problem hiding this comment.
We are probably going to do this change at some point but it doesn't make a lot of sense to do it on this PR, should probably do it in #1822
There was a problem hiding this comment.
reverted to go 1.13
joao-r-reis
changed the title
joao-r-reis
changed the title
Currently, the LZ4 compressor is maintained as a separate module under gocql/lz4. However, its implementation is tightly coupled with Cassandra's specific requirements. To streamline development, reduce dependency management complexity, and better encapsulate Cassandra-specific logic, the lz4 compressor has been moved to a lz4 package within the main gocql module. Patch by Mykyta Oleksiienko; reviewed by Joao Reis for CASSGO-32
OleksiienkoMykyta
force-pushed
the
CASSGO-32-lz4-has-been-moved-into-gocql-package
branch
from
efe9a1b
to
8957933
Compare
| @@ -10,6 +10,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 | |||
|
|
|||
| ### Changed | |||
|
|
|||
| - Move lz4 Compressor into gocql package (CASSGO-32) | |||
There was a problem hiding this comment.
I think it should be something like Move lz4 compressor to lz4 package within the gocql module
| github.com/golang/snappy v0.0.3 | ||
| github.com/hailocab/go-hostpool v0.0.0-20160125115350-e80d13ce29ed | ||
| github.com/kr/pretty v0.1.0 // indirect | ||
| github.com/stretchr/testify v1.3.0 // indirect | ||
| github.com/pierrec/lz4/v4 v4.1.8 |
There was a problem hiding this comment.
As I see from lz4 repo for release 4.1.8 go version in go.mod is set to 1.14, but in the gocql go.mod go version is set to 1.13.
Also, this PR bumps version of github.com/stretchr/testify from 1.7.0 to 1.9.0 that requires go of version at least 1.17, The 1.7.0 version only needs go 1.13.
@joao-r-reis should this PR change the go version here due to removing lz4 submodule?
There was a problem hiding this comment.
Hmm I don't think it matters if we change the version on go.mod here or not, the current version in go.mod is already incorrect and has been for a while anyway so I think we can just keep it unchanged in this PR and make the change on the v5 support PR to 1.19
This PR should be reviewed right after #1822 is merged.