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

backport: Use enum instead of int32 for BondStatus (#7499) #7516

Merged
merged 2 commits into from
Oct 19, 2020
Merged

backport: Use enum instead of int32 for BondStatus (#7499) #7516

merged 2 commits into from
Oct 19, 2020

Conversation

amaury1093
Copy link
Contributor

ref: #7464 (comment)

Backports:

@amaury1093 @blushi @alexanderbez
Use enum instead of int32 for BondStatus (#7499)
17eeff0 
* Migrate staking module

* Add gov legacy

* Add comments

* Add x/distrib

* x/crisis

* x/mint

* Fix test

* migrate x/genutil

* Fix lint

* Fix staking constants

* Fix test

* Update x/genutil/legacy/v040/migrate.go

Co-authored-by: Marie Gauthier <marie.gauthier63@gmail.com>

* Add migrate script instead of change BondStatus constants

* Change staking bondStatus to enum

* Fix test

* Fix another test

* Remove staking exported

* fix references

* Better constants

* Fix build

* Fix lint

* Remove buf version

* Fix tests

* Fix test

Co-authored-by: Marie Gauthier <marie.gauthier63@gmail.com>
Co-authored-by: Aleksandr Bezobchuk <alexanderbez@users.noreply.github.com>
@codecov
Copy link

codecov bot commented Oct 12, 2020 •
edited
Loading

Codecov Report

❗ No coverage uploaded for pull request base (v0.40.x@1532492). Click here to learn what that means.
The diff coverage is n/a.

@@            Coverage Diff             @@
##             v0.40.x    #7516   +/-   ##
==========================================
  Coverage           ?   56.02%           
==========================================
  Files              ?      592           
  Lines              ?    37243           
  Branches           ?        0           
==========================================
  Hits               ?    20865           
  Misses             ?    14261           
  Partials           ?     2117

@clevinson
Copy link
Contributor

@amaurymartiny I think its better if this changelog entry stay in an unreleased section, and then we can combine relevant entries & tag as an RC1 section when ready for the next release candidate.

@amaury1093
Copy link
Contributor Author

amaury1093 commented Oct 14, 2020 •
edited
Loading

Side note: This is a breaking change (even a proto-breaking change). I guess we're okay with breaking changes between RCs, right?

Since RC0 has been released with v1beta1, if we want to put this in another RC, I should probably update the proto version to v1beta2.

The alternative would be to wait 0.41 (and hopefully a .v1 version for most proto packages)

@aaronc
Copy link
Member

aaronc commented Oct 14, 2020

Side note: This is a breaking change (even a proto-breaking change). I guess we're okay with breaking changes between RCs, right?

Yes

Since RC0 has been released with v1beta1, if we want to put this in another RC, I should probably update the proto version to v1beta2.

I think that's okay, maybe not strictly necessary but makes sense

@alexanderbez
Copy link
Contributor

Breaking changes between RCs are totally acceptable -- they're release candidates

@aaronc aaronc added this to the v0.40 [Stargate] milestone Oct 15, 2020
@amaury1093 amaury1093 marked this pull request as draft October 16, 2020 10:00
@clevinson
Copy link
Contributor

@amaurymartiny Is this ready for review?

@clevinson clevinson mentioned this pull request Oct 19, 2020
Merged
31 tasks
@amaury1093 amaury1093 marked this pull request as ready for review October 19, 2020 10:26
@amaury1093
Copy link
Contributor Author

amaury1093 commented Oct 19, 2020 •
edited
Loading

@clevinson Yes. If you want to include this backport directly in #7590 to avoid merge conflicts, we can do that too.

@aaronc aaronc added the T: Proto Breaking Protobuf breaking changes: generally don't do this! label Oct 19, 2020
aaronc
aaronc approved these changes Oct 19, 2020
View reviewed changes
Copy link
Member

@aaronc aaronc left a comment

Choose a reason for hiding this comment

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

ACK

alexanderbez
alexanderbez approved these changes Oct 19, 2020
View reviewed changes
Copy link
Contributor

@alexanderbez alexanderbez left a comment

Choose a reason for hiding this comment

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

utACK

@clevinson
Copy link
Contributor

@amaurymartiny it will be good to get this merged independently of #7590, as the staking module changes in #7599 are dependent on this.

@clevinson clevinson merged commit 3dd64be into v0.40.x Oct 19, 2020
@clevinson clevinson deleted the am-v040x-backport branch October 19, 2020 19:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@aaronc aaronc aaronc approved these changes

@clevinson clevinson clevinson approved these changes

@alexanderbez alexanderbez alexanderbez approved these changes

@alessio alessio Awaiting requested review from alessio

Assignees
No one assigned
Labels
T: Proto Breaking Protobuf breaking changes: generally don't do this!
Projects
None yet
Milestone
v0.40 [Stargate]
Development

Successfully merging this pull request may close these issues.
4 participants
@amaury1093 @clevinson @aaronc @alexanderbez