Skip to content

algod: Rename enums by default#5089

Merged
winder merged 1 commit into
algorand:masterfrom
Eric-Warehime:rename-enum-conflicts
Feb 2, 2023
Merged

algod: Rename enums by default#5089
winder merged 1 commit into
algorand:masterfrom
Eric-Warehime:rename-enum-conflicts

Conversation

@Eric-Warehime
Copy link
Copy Markdown
Contributor

@algochoi noticed that some of the values for enums in our generated models had inconsistent naming. For example, https://github.com/algorand/go-algorand/blob/master/daemon/algod/api/server/v2/generated/model/types.go#L117 is named Json whereas all other parameters have their type name prefixed.

This is because we are not setting https://github.com/deepmap/oapi-codegen/blob/f4cf8f9a570380c24c6ba03ae04b9393cf120692/pkg/codegen/configuration.go#L66 AlwaysPrefixEnumValues. I've set that for our model and regenerated them, fixing issues that I saw come up because of it.

This means that every enum value will have its name prefixed, not just those where a naming conflict is detected.

@codecov
Copy link
Copy Markdown

codecov Bot commented Jan 31, 2023

Codecov Report

Merging #5089 (a8f3056) into master (8e5df38) will increase coverage by 0.02%.
The diff coverage is 0.00%.

@@            Coverage Diff             @@
##           master    #5089      +/-   ##
==========================================
+ Coverage   53.53%   53.56%   +0.02%     
==========================================
  Files         430      430              
  Lines       54086    54086              
==========================================
+ Hits        28954    28969      +15     
+ Misses      22880    22866      -14     
+ Partials     2252     2251       -1     
Impacted Files Coverage Δ
daemon/algod/api/server/v2/handlers.go 0.82% <0.00%> (ø)
daemon/algod/api/server/v2/utils.go 12.92% <0.00%> (ø)
ledger/testing/randomAccounts.go 56.26% <0.00%> (-0.62%) ⬇️
network/wsPeer.go 67.83% <0.00%> (+0.46%) ⬆️
ledger/acctupdates.go 69.24% <0.00%> (+0.48%) ⬆️
ledger/catchpointtracker.go 57.92% <0.00%> (+0.78%) ⬆️
catchup/service.go 69.80% <0.00%> (+1.20%) ⬆️

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

Copy link
Copy Markdown
Contributor

@algochoi algochoi left a comment

Choose a reason for hiding this comment

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

Thanks for this PR - I think this is a good improvement as changing existing type names when regenerating code seems undesirable and confusing. I also thing there is a nice improvement in getCodecHandle that will make it more robust to future changes. I didn't see any existing references to the other changed field names either (Acfg -> TxTypeAcfg), so I will leave a soft vote of approval.

@Eric-Warehime Eric-Warehime marked this pull request as ready for review February 1, 2023 16:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants