Skip to content

refactor: protobuf make targets for v0.50 #51

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.

Sign up for GitHub

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

Merged
damiannolan merged 6 commits into from
Mar 10, 2025
Merged

refactor: protobuf make targets for v0.50 #51

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
44f937b
refactor: protobuf make targets for v0.50
damiannolan Mar 7, 2025
5244712
chore: rm swagger until required
damiannolan Mar 7, 2025
ec246f9
Merge branch 'sdk-v0.50.x' into damian/proto-make-targets
damiannolan Mar 7, 2025
e0a0ba7
chore: rm comment in clang format file
damiannolan Mar 7, 2025
119d6d8
chore: use buf raw tooling, rm protobuilder docker
damiannolan Mar 7, 2025
bca0fcf
Merge branch 'sdk-v0.50.x' into damian/proto-make-targets
damiannolan Mar 10, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
115 changes: 115 additions & 0 deletions .clang-format
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,115 @@
---
Language: Proto
AccessModifierOffset: -2
AlignAfterOpenBracket: Align
AlignConsecutiveAssignments: true
AlignConsecutiveDeclarations: true
AlignEscapedNewlines: Right
AlignOperands: true
AlignTrailingComments: true
AllowAllParametersOfDeclarationOnNextLine: true
AllowShortBlocksOnASingleLine: true
AllowShortCaseLabelsOnASingleLine: false
AllowShortFunctionsOnASingleLine: Empty
AllowShortIfStatementsOnASingleLine: false
AllowShortLoopsOnASingleLine: false
AlwaysBreakAfterDefinitionReturnType: None
AlwaysBreakAfterReturnType: None
AlwaysBreakBeforeMultilineStrings: false
AlwaysBreakTemplateDeclarations: false
BinPackArguments: true
BinPackParameters: true
BraceWrapping:
AfterClass: false
AfterControlStatement: false
AfterEnum: false
AfterFunction: false
AfterNamespace: false
AfterObjCDeclaration: false
AfterStruct: false
AfterUnion: false
AfterExternBlock: false
BeforeCatch: false
BeforeElse: false
IndentBraces: false
SplitEmptyFunction: true
SplitEmptyRecord: true
SplitEmptyNamespace: true
BreakBeforeBinaryOperators: None
BreakBeforeBraces: Attach
BreakBeforeInheritanceComma: false
BreakBeforeTernaryOperators: true
BreakConstructorInitializersBeforeComma: false
BreakConstructorInitializers: BeforeColon
BreakAfterJavaFieldAnnotations: false
BreakStringLiterals: true
ColumnLimit: 120
CommentPragmas: "^ IWYU pragma:"
CompactNamespaces: false
ConstructorInitializerAllOnOneLineOrOnePerLine: false
ConstructorInitializerIndentWidth: 4
ContinuationIndentWidth: 4
Cpp11BracedListStyle: true
DerivePointerAlignment: false
DisableFormat: false
ExperimentalAutoDetectBinPacking: false
FixNamespaceComments: true
ForEachMacros:
- foreach
- Q_FOREACH
- BOOST_FOREACH
IncludeBlocks: Preserve
IncludeCategories:
- Regex: '^"(llvm|llvm-c|clang|clang-c)/'
Priority: 2
- Regex: '^(<|"(gtest|gmock|isl|json)/)'
Priority: 3
- Regex: ".*"
Priority: 1
IncludeIsMainRegex: "(Test)?$"
IndentCaseLabels: false
IndentPPDirectives: None
IndentWidth: 2
IndentWrappedFunctionNames: false
JavaScriptQuotes: Leave
JavaScriptWrapImports: true
KeepEmptyLinesAtTheStartOfBlocks: true
MacroBlockBegin: ""
MacroBlockEnd: ""
MaxEmptyLinesToKeep: 1
NamespaceIndentation: None
ObjCBlockIndentWidth: 2
ObjCSpaceAfterProperty: false
ObjCSpaceBeforeProtocolList: true
PenaltyBreakAssignment: 2
PenaltyBreakBeforeFirstCallParameter: 19
PenaltyBreakComment: 300
PenaltyBreakFirstLessLess: 120
PenaltyBreakString: 1000
PenaltyExcessCharacter: 1000000
PenaltyReturnTypeOnItsOwnLine: 60
PointerAlignment: Right
RawStringFormats:
- Delimiters:
- pb
Language: TextProto
BasedOnStyle: google
ReflowComments: true
SortIncludes: true
SortUsingDeclarations: true
SpaceAfterCStyleCast: false
SpaceAfterTemplateKeyword: true
SpaceBeforeAssignmentOperators: true
SpaceBeforeParens: ControlStatements
SpaceInEmptyParentheses: false
SpacesBeforeTrailingComments: 1
SpacesInAngles: false
SpacesInContainerLiterals: false
SpacesInCStyleCastParentheses: false
SpacesInParentheses: false
SpacesInSquareBrackets: false
Standard: Cpp11
TabWidth: 8
UseTab: Never
---

51 changes: 30 additions & 21 deletions Makefile
View file
Open in desktop
Copy link
Collaborator

@julienrbrt julienrbrt Mar 7, 2025

Choose a reason for hiding this comment

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

Any reason for the changes in this file actually?

Copy link
Author

@damiannolan damiannolan Mar 7, 2025

Choose a reason for hiding this comment

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

was just quickest to get things working by using docker image

Copy link
Collaborator

@julienrbrt julienrbrt Mar 7, 2025

Choose a reason for hiding this comment

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

but what was there should have been working, this is how I generated the protos when we started to upgrade from 0.46. What wasn't working 🤔

Copy link
Author

@damiannolan damiannolan Mar 7, 2025

Choose a reason for hiding this comment

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

I think proto-gen was failing due to me not having protoc-gen-gocosmos iirc, plus linting was complaining with something else - that's when I decided to take a look at things. Happy to use the tooling directly tho, things are good now

Original file line number Diff line number Diff line change
Expand Up @@ -53,44 +53,53 @@ mod-verify: mod
GO111MODULE=on go mod verify
.PHONY: mod-verify

# Use this target if you do not want to use Ignite for generating proto files
###############################################################################
### Protobuf ###
###############################################################################
BUF_VERSION=v1.50.0
GOLANG_PROTOBUF_VERSION=1.28.1
GRPC_GATEWAY_VERSION=1.16.0
GRPC_GATEWAY_PROTOC_GEN_OPENAPIV2_VERSION=2.20.0


#? proto-all: Format, lint and generate Protobuf files
proto-all: proto-deps proto-format proto-lint proto-gen

#? proto-deps: Install Protobuf local dependencies
proto-deps:
@echo "Installing proto deps"
Copy link
Collaborator

@julienrbrt julienrbrt Mar 7, 2025

Choose a reason for hiding this comment

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

Let's bring this back and not use proto builder. It can be a blocker if ICL doesn't maintain it well. Depending on the tools directly should be fine.

Copy link
Author

@damiannolan damiannolan Mar 7, 2025

Choose a reason for hiding this comment

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

I reverted to use raw tooling instead of docker image here 119d6d8

I had to add protoc-gen-gocosmos to list of deps and a few other tweaks to get things working, but seems good now

@go install github.com/bufbuild/buf/cmd/buf@v1.50.0
@go install github.com/cosmos/gogoproto/protoc-gen-gogo@latest
@go install github.com/bufbuild/buf/cmd/buf@$(BUF_VERSION)
@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/cosmos/gogoproto/protoc-gen-gocosmos@latest
@go install github.com/cosmos/gogoproto/protoc-gen-gogo@latest
@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 google.golang.org/grpc/cmd/protoc-gen-go-grpc@latest
@go install google.golang.org/protobuf/cmd/protoc-gen-go@v$(GOLANG_PROTOBUF_VERSION)

## proto-gen: Generate protobuf files. Requires docker.
proto-gen: proto-deps
@echo "--> Generating Protobuf files"
#? proto-gen: Generate Protobuf files
proto-gen:
@echo "Generating Protobuf files"
@sh ./scripts/protocgen.sh
.PHONY: proto-gen

## proto-lint: Lint protobuf files. Requires docker.
proto-lint: proto-deps
@echo "--> Linting Protobuf files"
#? proto-format: Format Protobuf files
proto-format:
@find ./ -name "*.proto" -exec clang-format -i {} \;
Copy link
Author

@damiannolan damiannolan Mar 7, 2025

Choose a reason for hiding this comment

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

should we add to makefile? :D

install-clang-format:
    @echo "Detecting OS..."
    @if [ "$(shell uname)" = "Darwin" ]; then \
	echo "Installing clang-format on macOS..."; \
	brew install clang-format; \
    elif [ "$(shell uname)" = "Linux" ]; then \
	echo "Installing clang-format on Linux..."; \
	sudo apt-get update && sudo apt-get install -y clang-format || \
	sudo yum install -y clang-tools-extra || \
	sudo dnf install -y clang-tools-extra;

Copy link
Collaborator

@chatton chatton Mar 7, 2025

Choose a reason for hiding this comment

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

I guess we can 😅 , bit ugly but if it's required. Might be nice to have one script that does platform based installation for all the tooling that's required. golanglint-ci, markdownlint, clang-format etc. But if this works out for now that's okay.

Copy link
Author

@damiannolan damiannolan Mar 7, 2025

Choose a reason for hiding this comment

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

Yeah, I mean, we can also just do without it for the time being and add one for all tooling if needs be. It's pretty obvious that you need to install it if its missing.


#? proto-lint: Lint Protobuf files
proto-lint:
@buf lint --error-format=json
.PHONY: proto-lint

## proto-check-breaking: Check if there are any breaking change to protobuf definitions.
proto-check-breaking: proto-deps
@echo "--> Checking if Protobuf definitions have any breaking changes"
#? proto-check-breaking: Check if Protobuf file contains breaking changes
proto-check-breaking:
@buf breaking --against $(HTTPS_GIT)#branch=main
.PHONY: proto-check-breaking

## proto-format: Format protobuf files. Requires Docker.
proto-format: proto-deps
@echo "--> Formatting Protobuf files"
@find ./ -name "*.proto" -exec clang-format -i {} \;
.PHONY: proto-format
#? proto-update-deps: Update Protobuf dependencies
proto-update-deps:
@echo "Updating Protobuf dependencies"
@cd proto && buf dep update

.PHONY: proto-all proto-deps proto-gen proto-format proto-lint proto-check-breaking proto-update-deps

build-docker:
@echo "--> Building Docker image"
Expand Down
134 changes: 65 additions & 69 deletions app/grpc/gasestimation/gas_estimator.pb.go
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

6 changes: 4 additions & 2 deletions app/grpc/tx/tx.pb.go
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Loading
Loading