-
Notifications
You must be signed in to change notification settings - Fork 3.6k
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
chore: update protoc-gen-swagger to protoc-gen-openapiv2 #20448
chore: update protoc-gen-swagger to protoc-gen-openapiv2 #20448
Conversation
WalkthroughWalkthroughThe recent updates primarily involve transitioning from the deprecated Changes
Assessment against linked issues
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (invoked as PR comments)
Additionally, you can add CodeRabbit Configration File (
|
@@ -4,6 +4,8 @@ package cosmos.app.v1alpha1; | |||
|
|||
import "google/protobuf/any.proto"; | |||
|
|||
option go_package = "cosmossdk.io/api/app/v1alpha1"; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Added this option since grpc-ecosystem/grpc-gateway#2127
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
Outside diff range and nitpick comments (5)
scripts/protoc-swagger-gen.sh (2)
Line range hint
11-11
: Refactor the condition check for clarity.- if [[ ! -z "$query_file" ]]; then + if [[ -n "$query_file" ]]; thenUsing
-n
directly is clearer and more idiomatic in bash than using! -z
.
Line range hint
12-12
: Ensure proper quoting to prevent word splitting.- buf generate --template buf.gen.swagger.yaml $query_file + buf generate --template buf.gen.swagger.yaml "$query_file"This change prevents potential issues when the path contains spaces or special characters.
contrib/devtools/Dockerfile (3)
Line range hint
8-8
: Pin versions for APK packages to ensure reproducibility.- apk add --no-cache \ + apk add --no-cache \ + nodejs=<version> \ + npm=<version> \ + git=<version> \ + make=<version> \ + clang-extra-tools=<version> \ + g++=<version> \ + jq=<version>Pinning versions helps avoid unexpected changes due to package updates and ensures that the build environment is consistent and reproducible.
Line range hint
17-17
: ConsolidateRUN
instructions to optimize Docker layers.- RUN npm install -g swagger-combine - RUN adduser -u $UID -s /bin/sh $UNAME -D + RUN npm install -g swagger-combine && \ + adduser -u $UID -s /bin/sh $UNAME -DConsolidating these commands into a single
RUN
instruction reduces the number of layers in the Docker image, which can improve build performance and reduce the image size.
Line range hint
35-35
: UseWORKDIR
instead ofcd
.- COPY --from=BUILDER /usr/local/bin /usr/local/bin - RUN cd gogoproto; \ + COPY --from=BUILDER /usr/local/bin /usr/local/bin + WORKDIR gogoproto + RUN make installUsing
WORKDIR
is a cleaner and more error-proof method of changing directories in a Dockerfile.
Review Details
Configuration used: .coderabbit.yml
Review profile: CHILL
Files selected for processing (7)
- contrib/devtools/Dockerfile (1 hunks)
- proto/buf.gen.swagger.yaml (1 hunks)
- proto/cosmos/app/v1alpha1/config.proto (1 hunks)
- proto/cosmos/app/v1alpha1/module.proto (1 hunks)
- proto/cosmos/app/v1alpha1/query.proto (1 hunks)
- proto/cosmos/orm/query/v1alpha1/query.proto (1 hunks)
- scripts/protoc-swagger-gen.sh (1 hunks)
Files skipped from review due to trivial changes (5)
- proto/buf.gen.swagger.yaml
- proto/cosmos/app/v1alpha1/config.proto
- proto/cosmos/app/v1alpha1/module.proto
- proto/cosmos/app/v1alpha1/query.proto
- proto/cosmos/orm/query/v1alpha1/query.proto
Additional Context Used
ShellCheck (2)
scripts/protoc-swagger-gen.sh (2)
[style] 11-11: Use -n instead of ! -z.
[info] 12-12: Double quote to prevent globbing and word splitting.
Hadolint (5)
contrib/devtools/Dockerfile (5)
8: Pin versions in apk add. Instead of
apk add <package>
useapk add <package>=<version>
17: Multiple consecutive
RUN
instructions. Consider consolidation.
17: Pin versions in npm. Instead of
npm install <package>
usenpm install <package>@<version>
35: Use WORKDIR to switch to a directory
35: Use 'cd ... || exit' or 'cd ... || return' in case cd fails.
Additional comments not posted (2)
scripts/protoc-swagger-gen.sh (1)
7-7
: Expand search for.proto
files to include the../x
directory.This change allows the script to consider additional directories for
.proto
files, which can be beneficial for projects that have expanded their structure.contrib/devtools/Dockerfile (1)
29-30
: Update the installation command to use the newprotoc-gen-openapiv2
.This change aligns with the project's transition from Swagger to OpenAPIv2, ensuring compatibility with the latest versions of the grpc-gateway.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
Outside diff range and nitpick comments (3)
contrib/devtools/Dockerfile (3)
Line range hint
8-8
: Pin versions of packages inapk add
andnpm install
commands to ensure reproducibility and avoid potential breaks due to updates.- RUN apk add --no-cache nodejs npm git make clang-extra-tools g++ + RUN apk add --no-cache nodejs=<version> npm=<version> git=<version> make=<version> clang-extra-tools=<version> g++=<version> - RUN npm install -g swagger-combine + RUN npm install -g swagger-combine@<specific_version>Also applies to: 17-17
Line range hint
17-17
: Consider consolidating multipleRUN
instructions to reduce the number of layers in the Docker image, which can improve build performance and reduce the image size.- RUN go install github.com/cosmos/cosmos-proto/cmd/protoc-gen-go-pulsar@latest && \ - go install google.golang.org/protobuf/cmd/protoc-gen-go@v${GOLANG_PROTOBUF_VERSION} && \ - go install github.com/grpc-ecosystem/grpc-gateway/protoc-gen-grpc-gateway@v${GRPC_GATEWAY_VERSION} && \ - go install github.com/grpc-ecosystem/grpc-gateway/v2/protoc-gen-openapiv2@v${GRPC_GATEWAY_PROTOC_GEN_OPENAPIV2_VERSION} && \ - go install cosmossdk.io/orm/cmd/[email protected] && \ - go install google.golang.org/grpc/cmd/protoc-gen-go-grpc@latest + RUN go install github.com/cosmos/cosmos-proto/cmd/protoc-gen-go-pulsar@latest && \ + go install google.golang.org/protobuf/cmd/protoc-gen-go@v${GOLANG_PROTOBUF_VERSION} && \ + go install github.com/grpc-ecosystem/grpc-gateway/protoc-gen-grpc-gateway@v${GRPC_GATEWAY_VERSION} && \ + go install github.com/grpc-ecosystem/grpc-gateway/v2/protoc-gen-openapiv2@v${GRPC_GATEWAY_PROTOC_GEN_OPENAPIV2_VERSION} && \ + go install cosmossdk.io/orm/cmd/[email protected] && \ + go install google.golang.org/grpc/cmd/protoc-gen-go-grpc@latest
Line range hint
36-36
: Add error handling forcd
to ensure subsequent commands run in the intended directory.- RUN git clone https://github.com/cosmos/gogoproto.git; \ - cd gogoproto; \ + RUN git clone https://github.com/cosmos/gogoproto.git && cd gogoproto || exit; \
Review details
Configuration used: .coderabbit.yml
Review profile: CHILL
Files selected for processing (1)
- contrib/devtools/Dockerfile (1 hunks)
Additional context used
Hadolint
contrib/devtools/Dockerfile
[warning] 8-8: Pin versions in apk add. Instead of
apk add <package>
useapk add <package>=<version>
[info] 17-17: Multiple consecutive
RUN
instructions. Consider consolidation.
[warning] 17-17: Pin versions in npm. Instead of
npm install <package>
usenpm install <package>@<version>
[warning] 36-36: Use WORKDIR to switch to a directory
[warning] 36-36: Use 'cd ... || exit' or 'cd ... || return' in case cd fails.
Additional comments not posted (1)
contrib/devtools/Dockerfile (1)
31-31
: Update toprotoc-gen-openapiv2
aligns with the latest GRPC Gateway standards.
* main: (120 commits) chore: update protoc-gen-swagger to protoc-gen-openapiv2 (#20448) ci: Add GitHub Action for go mod tidy and mocks (#20501) chore: Address linter issues (#20486) fix: wrap errors in auto CLI service registration (#20493) chore: fix comment (#20498) chore: fix the note box syntax error (#20499) feat(server/v2): introduce cometbft v2 (#20483) refactor(x/upgrade): migrate to appmodule.VersionMap (#20485) docs: code guidelines changes (#20482) feat(cosmovisor): load cosmovisor configuration from toml file (#19995) perf(math): Significantly speedup Dec quo truncate and quo Roundup (#20034) fix: Bump CometBFT versions (#20481) refactor(core): remove redundant ExecMode (#20322) feat(store/v2): pruning manager (#20430) chore: force reload sonar cloud (#20480) refactor(x/accounts): reuse calculated sum in `func safeAdd` (#20458) refactor: remove `defer` in loop (#20223) ci: remove livness test (#20474) build(deps): Bump bufbuild/buf-setup-action from 1.32.1 to 1.32.2 (#20477) chore: migrate a few diagrams to mermaid (#20473) ...
Description
Closes: #7620
This PR updates
protoc-gen-swagger
plugin intoprotoc-gen-openapiv2
plugin to have properprotobuf.Any
structure in generated swagger file.Author Checklist
All items are required. Please add a note to the item if the item is not applicable and
please add links to any relevant follow up issues.
I have...
!
in the type prefix if API or client breaking changeCHANGELOG.md
Reviewers Checklist
All items are required. Please add a note if the item is not applicable and please add
your handle next to the items reviewed if you only reviewed selected items.
I have...
Summary by CodeRabbit
New Features
protoc-gen-openapiv2
instead ofprotoc-gen-swagger
for better OpenAPIv2 support.Configuration Updates
go_package
directives in multiple.proto
files to specify Go package details.Scripts
.proto
files.