Skip to content

fix: temporarily disable new historacle proto#1586

Closed
toteki wants to merge 4 commits intomainfrom
adam/undo
Closed

fix: temporarily disable new historacle proto#1586
toteki wants to merge 4 commits intomainfrom
adam/undo

Conversation

@toteki
Copy link
Copy Markdown
Contributor

@toteki toteki commented Nov 14, 2022

Description

First step of #1585 - second would be re-enabling.

I think this is a cleaner path to v3.2.0 release than leaving main branch incompatible or building a migration to put the params in early.

@toteki toteki marked this pull request as ready for review November 14, 2022 16:44
@toteki toteki requested review from a team as code owners November 14, 2022 16:44
@adamewozniak
Copy link
Copy Markdown
Collaborator

imo this is the cleaner way to go.
cc @zarazan @rbajollari

@adamewozniak
Copy link
Copy Markdown
Collaborator

adamewozniak commented Nov 14, 2022

@toteki after this PR is merged will we be g2g on the gravity bridge upgrade?
cc: @robert-zaremba

@toteki
Copy link
Copy Markdown
Contributor Author

toteki commented Nov 14, 2022

This and #1577 yeah

@robert-zaremba
Copy link
Copy Markdown
Contributor

I'm going sleep now. Personally i think it's better to add the fix rather then doing 3 similar commits (old push, this revert, and the new re-push with fix). Of course this only make sense if we are not under big pressure.l, and i already have most of the gov message part done. Need to update CLI .

@robert-zaremba
Copy link
Copy Markdown
Contributor

That being said I'm not against revert and redo if others support it.

Copy link
Copy Markdown
Contributor

@robert-zaremba robert-zaremba left a comment

Choose a reason for hiding this comment

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

As noted in other comment, we don't need to go back and forth with these proto changes. So blocking it because these are unnecessary back-and-forth changes.

I've prepared a migration: #1593
still need to test it.

The full migration to go out of the param space is a bit more complex, so will be better to do it in v3.3

@adamewozniak
Copy link
Copy Markdown
Collaborator

@toteki can this be closed in favor of #1586 ?

@toteki toteki closed this Nov 17, 2022
@toteki toteki deleted the adam/undo branch November 17, 2022 20:35
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.

4 participants