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

Bumps #3999

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

Bumps #3999

wants to merge 8 commits into from

Conversation

geyslan
Copy link
Member

@geyslan geyslan commented Apr 23, 2024

1. Explain what the PR does

cdf51f4 chore(deps): bump go to 1.22.3
015e29b chore(builder): bump pandoc to 3.2
5bfb528 chore(builder): pin protoc versions
5170c47 chore: use NewClient instead of Dial (deprecated)
08ab832 chore: bump go.mod and go.sum
01c3539 chore(signatures): bump go.mod and go.sum
f6ccb30 chore(types): bump go.mod and go.sum
45bb393 chore(api): bump go.mod and go.sum

015e29b chore(builder): bump pandoc to 3.2

It also updates all man by generating them with the new pandoc version.

5bfb528 chore(builder): pin protoc versions

This doesn't upgrade versions to avoid changes in the protoc output
which would require updating the generated code.

5170c47 chore: use NewClient instead of Dial (deprecated)

grpc pkg introduced NewClient in v1.63.0.

SA1019  grpc.Dial is deprecated: use NewClient instead.  Will be supported throughout 1.x.

2. Explain how to test it

3. Other comments

@geyslan

This comment was marked as resolved.

@geyslan geyslan force-pushed the bumps branch 2 times, most recently from ac8e0e4 to 5cdcf8a Compare May 17, 2024 14:40
@geyslan geyslan marked this pull request as ready for review May 17, 2024 16:53
@geyslan
Copy link
Member Author

geyslan commented May 17, 2024

After this be merged. A sequential PR should be created bumping the main go.mod, specifically:

github.com/aquasecurity/tracee/api
github.com/aquasecurity/tracee/signatures/helpers
github.com/aquasecurity/tracee/types

So, only after that, we should update internal tests.

@geyslan
Copy link
Member Author

geyslan commented May 17, 2024

@josedonizetti you may notice in f2d6129 that I pinned the protoc version to ensure that the generated files are not modified. But I would like to know if there is no synchronization problem with the client side when updating the versions of these files. If there's, what should be the steps to take? Or would you take care of bumping the protoc in a different PR?

api/go.mod Outdated
@@ -1,16 +1,15 @@
module github.com/aquasecurity/tracee/api

go 1.21
go 1.22.3
Copy link
Collaborator

Choose a reason for hiding this comment

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

Maybe a silly question, but why do we need to specify the patch version? won't 1.22 be enough?

Copy link
Member Author

@geyslan geyslan May 20, 2024

Choose a reason for hiding this comment

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

Good point. I should be using the toolchain directive here like:

go 1.22.0              // doesn't enforce any specific patch version, but doesn't limit it to 1.22.0 as minor.

toolchain go1.22.3     // do enforce this specific patch version

It's sane to make the whole environment to use the same version (patch or not).

Copy link
Contributor

@rscampos rscampos left a comment

Choose a reason for hiding this comment

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

LGTM

@geyslan
Copy link
Member Author

geyslan commented May 20, 2024

@rscampos Thanks for reviewing it. I'm holding it for a while since it's good to have @NDStrahilevitz and @josedonizetti feedback as well, since it will demand changes to internal tests and beyond.

types/go.mod Show resolved Hide resolved
builder/Dockerfile.protoc Outdated Show resolved Hide resolved
This doesn't upgrade versions to avoid changes in the protoc output
which would require updating the generated code.
It also updates all man by generating them with the new pandoc version.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants