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.

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

feat!: Remove support for legacy plain text proposals #1668

Merged
merged 6 commits into from
Apr 28, 2023

Conversation

evan-forbes
Copy link
Member

@evan-forbes evan-forbes commented Apr 27, 2023

Overview

This PR removes legacy text proposals. While it is still possible to submit other proposals that are functionally very similar, here we are being explicit by removing the ability to handle text proposals.

part of #1593

Checklist

  • New and updated code has appropriate documentation
  • New and updated code has new and/or updated testing
  • Required CI checks are passing
  • Visual proof for any user facing features like CLI or documentation updates
  • Linked issues closed with keywords

@evan-forbes evan-forbes added this to the Mainnet milestone Apr 27, 2023
@evan-forbes evan-forbes self-assigned this Apr 27, 2023
@MSevey MSevey requested a review from a team April 27, 2023 13:02
@codecov-commenter
Copy link

codecov-commenter commented Apr 27, 2023

Codecov Report

Merging #1668 (73e7a2f) into main (4c73824) will increase coverage by 0.00%.
The diff coverage is 0.00%.

@@           Coverage Diff           @@
##             main    #1668   +/-   ##
=======================================
  Coverage   51.56%   51.57%           
=======================================
  Files          95       95           
  Lines        5954     5953    -1     
=======================================
  Hits         3070     3070           
+ Misses       2570     2569    -1     
  Partials      314      314           
Impacted Files Coverage Δ
app/app.go 4.66% <0.00%> (+0.01%) ⬆️

cmwaters
cmwaters previously approved these changes Apr 27, 2023
expectedCode: abci.CodeTypeOK,
// plain text proposals have been removed, so we expect an error. "No
// handler exists for proposal type"
expectedCode: uint32(9),
Copy link
Member

Choose a reason for hiding this comment

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

is there no specific CodeType that can be used instead of uint32(9)?

Copy link
Member Author

Choose a reason for hiding this comment

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

good point! 976b0bb

rach-id
rach-id previously approved these changes Apr 27, 2023
@evan-forbes evan-forbes dismissed stale reviews from rach-id and cmwaters via 976b0bb April 27, 2023 13:57
@MSevey MSevey requested a review from a team April 27, 2023 13:58
@evan-forbes evan-forbes enabled auto-merge (squash) April 27, 2023 15:08
Copy link
Collaborator

@rootulp rootulp left a comment

Choose a reason for hiding this comment

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

LGTM with one comment. Will approve after you get a chance to view it @evan-forbes so that this doesn't auto-merge

app/test/std_sdk_test.go Outdated Show resolved Hide resolved
@MSevey MSevey requested a review from a team April 27, 2023 15:41
@evan-forbes evan-forbes merged commit 32246ed into main Apr 28, 2023
27 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants