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.

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

Jump to bottom

Remove temporary import hack for protobuf any #744

Closed
Tracked by #301
rootulp opened this issue Sep 20, 2022 · 3 comments · Fixed by #2846
Closed
Tracked by #301

Remove temporary import hack for protobuf any #744

rootulp opened this issue Sep 20, 2022 · 3 comments · Fixed by #2846
Assignees
@rootulp
Labels
bug Something isn't working good first issue Good for newcomers

Comments

@rootulp
Copy link
Collaborator

rootulp commented Sep 20, 2022

Context

Problem

We want to remove the temporary import hack in https://github.com/celestiaorg/celestia-app/pull/742/files#diff-63113d53e02a4e334ffa1a1ba0c308329c7acd431153571a2c02ce9268af5816R28-R32
This was referenced Sep 20, 2022
Closed
Closed
@rach-id rach-id mentioned this issue Sep 20, 2022
Merged
@evan-forbes evan-forbes added bug Something isn't working C: QGB labels Oct 3, 2022
@rach-id rach-id added the good first issue Good for newcomers label Oct 4, 2022
@evan-forbes
Copy link
Member

is this still a thing @sweexordious ?

@rach-id
Copy link
Member

rach-id commented Nov 12, 2023

it's still here:

celestia-app/scripts/protocgen.sh

Lines 26 to 29 in b8168ac

# temporary import hack to use cosmos-sdk implementation of Any type.
# check https://github.com/celestiaorg/celestia-app/issues/507 for more information.
sed -i 's/types "github.com\/celestiaorg\/celestia-app\/codec\/types"/types "github.com\/cosmos\/cosmos-sdk\/codec\/types"/g' \
github.com/celestiaorg/celestia-app/x/blobstream/types/query.pb.go

not sure if it's worth fixing tho

@rootulp
Copy link
Collaborator Author

rootulp commented Nov 15, 2023

I think this issue is worth fixing. Ideally we remove the lines that perform the hack b/c it looks like they modify the generated query.pb.go file and the top of that file says:

celestia-app/x/mint/types/query.pb.go

Line 1 in b75c7e3

// Code generated by protoc-gen-gogo. DO NOT EDIT.

@rootulp rootulp self-assigned this Nov 15, 2023
@rootulp rootulp mentioned this issue Nov 15, 2023
Merged
rootulp added a commit that referenced this issue Nov 16, 2023
@rootulp
chore: remove import hack for protobuf any (#2846)
9b5d025 
Closes #744

This import hack doesn't seem necessary b/c it doesn't actually modify
`x/blobstream/types/query.pb.go`

## Testing

```
$ rm x/blobstream/types/query.pb.go
$ make proto-gen
```

and then observe that there is no `git diff` so no changes to the
already checked in `x/blobstream/types/query.pb.go` and the generated
one.

Also proto-gen passes in CI:
https://github.com/celestiaorg/celestia-app/actions/runs/6881660922/job/18718509598?pr=2846
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@rootulp rootulp

Labels
bug Something isn't working good first issue Good for newcomers
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

chore: remove import hack for protobuf any rootulp/celestia-app
3 participants
@rootulp @rach-id @evan-forbes