-
Notifications
You must be signed in to change notification settings - Fork 48
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
fix: fix swagger page and ci #1333
base: master
Are you sure you want to change the base?
Conversation
WalkthroughThe recent changes focus on enhancing the management of Protobuf dependencies and Swagger file generation for Cosmos SDK and IBC protocols. This includes updates to the Changes
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 (
|
SWAGGER_THIRD_PARTY_DIR=$(SWAGGER_PROTO_DIR)/third_party | ||
SWAGGER_COSMOS_SDK_BRANCH=release/v0.47.x | ||
SWAGGER_IBC_BRANCH=release/v7.4.x | ||
proto-download-deps: |
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.
To include cosmos-sdk and other third party queries and services into swagger, it requires download the raw proto files for generating proper swagger files.
@@ -110,6 +110,8 @@ import ( | |||
"github.com/CosmWasm/wasmd/x/wasm" | |||
wasmkeeper "github.com/CosmWasm/wasmd/x/wasm/keeper" | |||
wasmtypes "github.com/CosmWasm/wasmd/x/wasm/types" | |||
|
|||
_ "github.com/desmos-labs/desmos/v7/client/docs/statik" |
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.
Overwrite the swagger static files.
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
Out of diff range and nitpick comments (1)
scripts/protoc-swagger-gen.sh (1)
Line range hint
22-27
: Consider simplifying the condition check and ensure proper handling of variables to prevent issues like word splitting.- if [[ ! -z "$query_file" ]]; then + if [[ -n "$query_file" ]]; then - buf generate --template buf.gen.swagger.yaml $query_file + buf generate --template buf.gen.swagger.yaml "$query_file"
Review Details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files ignored due to path filters (9)
client/docs/swagger-ui/favicon-16x16.png
is excluded by!**/*.png
client/docs/swagger-ui/favicon-32x32.png
is excluded by!**/*.png
client/docs/swagger-ui/swagger-ui-bundle.js.map
is excluded by!**/*.map
client/docs/swagger-ui/swagger-ui-es-bundle-core.js.map
is excluded by!**/*.map
client/docs/swagger-ui/swagger-ui-es-bundle.js.map
is excluded by!**/*.map
client/docs/swagger-ui/swagger-ui-standalone-preset.js.map
is excluded by!**/*.map
client/docs/swagger-ui/swagger-ui.css.map
is excluded by!**/*.map
client/docs/swagger-ui/swagger-ui.js.map
is excluded by!**/*.map
proto/buf.lock
is excluded by!**/*.lock
Files selected for processing (6)
- Makefile (1 hunks)
- app/app.go (1 hunks)
- client/docs/config.json (1 hunks)
- client/docs/swagger-ui/index.html (1 hunks)
- client/docs/swagger-ui/oauth2-redirect.html (1 hunks)
- scripts/protoc-swagger-gen.sh (2 hunks)
Files not reviewed due to errors (3)
- client/docs/swagger-ui/oauth2-redirect.html (no review received)
- client/docs/config.json (no review received)
- Makefile (no review received)
Files skipped from review due to trivial changes (1)
- client/docs/swagger-ui/index.html
Additional Context Used
ShellCheck (2)
scripts/protoc-swagger-gen.sh (2)
[style] 26-26: Use -n instead of ! -z.
[info] 27-27: Double quote to prevent globbing and word splitting.
Additional comments not posted (7)
scripts/protoc-swagger-gen.sh (6)
6-10
: LGTM! Preparation steps for Swagger generation are well-structured.
11-15
: LGTM! Copying proto files to a new directory ensures isolation during Swagger generation.
16-16
: LGTM! Using a temporary folder for intermediate results is a good practice.
32-32
: LGTM! Cleaning up after generation prevents issues with stale files.
32-32
: LGTM! Effective finalization of the Swagger generation process.
Line range hint
6-32
: The script is well-structured and effectively supports the enhancements to Swagger documentation.app/app.go (1)
114-114
: Ensure the imported Swagger documentation is utilized within the application.Verification successful
The search results indicate that the import statement for the
statik
package appears only once inapp/app.go
, and that is the import line itself. There is no further evidence from the provided output that the imported package is being utilized elsewhere in the file. This could suggest that the import might be unnecessary unless it is implicitly used by the Go toolchain (which is common for packages that embed static files).To confirm whether this package is indeed used implicitly (which is a common pattern for packages that serve static files or documentation), I will need to check if there is any reference in the build scripts or other parts of the application where this package's content is accessed indirectly.
The search results from the scripts provide clear evidence that the
statik
package is indeed used within the project, particularly in the build process as specified in theMakefile
. The references tostatik
in theMakefile
and other related files indicate that it is used to embed Swagger UI assets into the Go binary. This is a common practice for serving static files directly from a Go application, which aligns with the purpose of enhancing Swagger documentation as mentioned in the PR.Additionally, the presence of a
statik.go
file in theclient/docs/statik
directory, which is generated by thestatik
tool and contains static assets, confirms that thestatik
package is not only imported but actively used to serve embedded content.Given this information, the import statement in
app/app.go
is justified and necessary for the application's functionality related to serving Swagger documentation. Therefore, the review comment questioning the utilization of the imported Swagger documentation is addressed, and the usage is verified as correct.Ensure the imported Swagger documentation is utilized within the application.
- The
statik
package is used in the build scripts to embed Swagger UI assets into the Go binary.- The generated
statik.go
file in theclient/docs/statik
directory confirms the active use of thestatik
package for serving embedded content.Scripts Executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the usage of the imported Swagger documentation package. # Test: Search for the usage of the `statik` package. Expect: At least one occurrence. rg --type go $'github.com/desmos-labs/desmos/v7/client/docs/statik' app/app.goLength of output: 134
Script:
#!/bin/bash # Description: Check for implicit usage of the `statik` package in build scripts or other Go files. # Search for any reference to the `statik` package in all Go files and build scripts. rg --type go --type make --type sh 'statik' .Length of output: 1571
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
Review Details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (1)
- .changeset/entries/9769bdf3cddefa33b7cbb5e13147c6b1f7a0d4193acb78e6904a7c1734fa5e68.yaml (1 hunks)
Files skipped from review due to trivial changes (1)
- .changeset/entries/9769bdf3cddefa33b7cbb5e13147c6b1f7a0d4193acb78e6904a7c1734fa5e68.yaml
Description
Closes: #XXXX
This PR fixes swagger page to show proper frontend and includes cosmos-sdk queries and services to 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...
!
to 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...
!
in the type prefix if API or client breaking changeSummary by CodeRabbit