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

fix: remove rosetta from server start #14062

Merged
merged 5 commits into from
Nov 30, 2022
Merged

fix: remove rosetta from server start #14062

merged 5 commits into from
Nov 30, 2022

Conversation

JulianToledano
Copy link
Contributor

@JulianToledano JulianToledano commented Nov 29, 2022 •
edited
Loading

Description

Closes:
#14041

Author Checklist

All items are required. Please add a note to the item if the item is not applicable and
please add links to any relevant follow up issues.

I have...

  • included the correct type prefix in the PR title
  • added ! to the type prefix if API or client breaking change
  • targeted the correct branch (see PR Targeting)
  • provided a link to the relevant issue or specification
  • followed the guidelines for building modules
  • included the necessary unit and integration tests
  • added a changelog entry to CHANGELOG.md
  • included comments for documenting Go code
  • updated the relevant documentation or specification
  • reviewed "Files changed" and left comments if necessary
  • confirmed all CI checks have passed

Reviewers Checklist

All items are required. Please add a note if the item is not applicable and please add
your handle next to the items reviewed if you only reviewed selected items.

I have...

  • confirmed the correct type prefix in the PR title
  • confirmed ! in the type prefix if API or client breaking change
  • confirmed all author checklist items have been addressed
  • reviewed state machine logic
  • reviewed API design and naming
  • reviewed documentation is accurate
  • reviewed tests and test coverage
  • manually tested (if applicable)

@JulianToledano JulianToledano changed the title refactor: remove rosetta from server start fix: remove rosetta from server start Nov 29, 2022
@JulianToledano JulianToledano marked this pull request as ready for review November 29, 2022 10:50
@JulianToledano JulianToledano requested a review from a team as a code owner November 29, 2022 10:50
@julienrbrt julienrbrt linked an issue Nov 29, 2022 that may be closed by this pull request
rosetta: break cyclical dependency with cosmos-sdk #14041
Closed
@odeke-em
Copy link
Collaborator

@JulianToledano thank you for this change! Please rebase the latest code from main and update this PR.

@JulianToledano JulianToledano force-pushed the fix/rosetta-cyclical-dependency branch 2 times, most recently from ddb9833 to fa1b73c Compare November 29, 2022 14:30
alexanderbez
alexanderbez approved these changes Nov 29, 2022
View reviewed changes
CHANGELOG.md Outdated
@@ -186,7 +186,7 @@ extension interfaces. `module.Manager.Modules` is now of type `map[string]interf
* (tx) [#12659](https://github.com/cosmos/cosmos-sdk/pull/12659) Remove broadcast mode `block`.

### Bug Fixes
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
### Bug Fixes
### Bug Fixes

@alexanderbez
Copy link
Contributor

TBH, I'm not sure this belongs under Bug Fix. Perhaps Improvement. We also need to mention it in breaking changes due to the config struct changing.

@JulianToledano
Copy link
Contributor Author

TBH, I'm not sure this belongs under Bug Fix. Perhaps Improvement. We also need to mention it in breaking changes due to the config struct changing.

you mean in the api breaking section?

@JulianToledano JulianToledano force-pushed the fix/rosetta-cyclical-dependency branch from 4669c01 to ee474da Compare November 30, 2022 08:41
@JulianToledano JulianToledano force-pushed the fix/rosetta-cyclical-dependency branch from ee474da to c80e09b Compare November 30, 2022 08:45
@tac0turtle
Copy link
Member

@JulianToledano could you enable the maintainers to update your branch to latest?otherwise we are playing a chasing game with you updating the pr

@tac0turtle tac0turtle enabled auto-merge (squash) November 30, 2022 10:56
@raynaudoe
Copy link
Contributor

@JulianToledano could you enable the maintainers to update your branch to latest?otherwise we are playing a chasing game with you updating the pr

Should we add you as collaborators ? or how can we give you guys that permission ?

@tac0turtle tac0turtle merged commit 6679ee0 into cosmos:main Nov 30, 2022
@JulianToledano JulianToledano deleted the fix/rosetta-cyclical-dependency branch December 13, 2022 10:23
@tkxkd0159 tkxkd0159 mentioned this pull request Jun 12, 2024
Closed
4 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@julienrbrt julienrbrt julienrbrt left review comments

@tac0turtle tac0turtle tac0turtle approved these changes

@facundomedica facundomedica facundomedica left review comments

@alexanderbez alexanderbez alexanderbez approved these changes

@odeke-em odeke-em odeke-em approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

rosetta: break cyclical dependency with cosmos-sdk
7 participants
@JulianToledano @odeke-em @alexanderbez @tac0turtle @raynaudoe @facundomedica @julienrbrt