Move messages only used to generate Go structs out of minder.proto #3830
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
Hold this - @dmjb had a good point that we should try getting rid of the structs first and foremost. |
I think we can actually review and merge this PR. @dmjb mentioned he wanted to get rid of the structs which is a good idea, but we'll need the internal proto file to include the selector messages anyway (the CEL evaluator works much easier with protobuf messages). It doesn't really matter whether we remove the messages that this PR moves to the internal proto file from the original proto file or the internal one and having the internal proto file created would enable us to move the selector messages there. |
dmjb
previously approved these changes
Jul 11, 2024
Adds a new proto file under the internal/ directory tree and modifies the `buf.work.yaml` file so that the new internal/proto directory is a module. Resolves: mindersec#3829
Uses the newly added Go structs that are generated out of the internal.proto file.
…file This commit just removes the now no longer used messages from the public minder.v1 module.
blkt
approved these changes
Jul 11, 2024
dmjb
pushed a commit
that referenced
this pull request
Jul 12, 2024
…3830) * Move protobuf messages only used internal to an internal proto file Adds a new proto file under the internal/ directory tree and modifies the `buf.work.yaml` file so that the new internal/proto directory is a module. Resolves: #3829 * Flip Minder code to use the Go structs from the internal proto file Uses the newly added Go structs that are generated out of the internal.proto file. * Remove the structs we moved to internal proto file from the v1 proto file This commit just removes the now no longer used messages from the public minder.v1 module. * Only check proto files in the proto subdir for backwards compatibility * Exclude internal/proto from code coverage
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary
This was brought up in PR #3797. We already have several messages that only
exist to generate Go structures out of them and use them with reflection. When
we add the profile selector messages, we'll have even more and our proto file
is going to become messy.
Instead, let's split out a new proto file and use it to store all the messages
that are really just a way to express Go structs.
Fixes: #3829
Change Type
Testing
make test
Review Checklist: