-
Notifications
You must be signed in to change notification settings - Fork 156
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
Bump module to major version 5 #2404
Conversation
|
||
go 1.20 | ||
go 1.22 |
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.
this can be debated for the ages but this is the minimum version we do support and now is as good of a time as any to bump it. consumers won't automatically be pulled into a newer go version if they just upgrade a v4 dcrwallet, it requires manual upgrading to v5.
@@ -2,7 +2,7 @@ syntax = "proto3"; | |||
|
|||
package walletrpc; | |||
|
|||
option go_package = "decred.org/dcrwallet/rpc/walletrpc"; | |||
option go_package = "decred.org/dcrwallet/v5/rpc/walletrpc"; |
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.
anyone know if this is used at all by anything? kinda weird how anything did before if so
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.
https://protobuf.dev/reference/go/go-generated/#package
The Go import path is locally specified in a .proto file by declaring a go_package option
the package name will be derived by default from the import path in a reasonable manner.
The import path is used to determine which import statements must be generated when one .proto file imports another .proto file.
Because dcrwallet doesnt have any proto files importing other proto files, it seems like the whole path is not used. Only the package name (the bit after the final slash) is used.
Verified this locally by setting option go_package = "bullshit/nonsense/walletrpc"
and running regen.sh, nothing changes. option go_package = "bullshit/nonsense/walletrpc2"
updates the package name of the generated files to "walletrpc2"
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.
Given the above, it seems like your suggested change is technically correct.
No description provided.