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

[skip changelog] Fix core commands skipping gRPC interface #1169

Merged
merged 1 commit into from Feb 5, 2021

Conversation

silvanocerza
Copy link
Contributor

Please check if the PR fulfills these requirements

  • The PR has no duplicates (please search among the Pull Requests
    before creating one)
  • The PR follows our contributing guidelines
  • Tests for the changes have been added (for bug fixes / features)
  • Docs have been added / updated (for bug fixes / features)
  • UPGRADING.md has been updated with a migration guide (for breaking changes)
  • What kind of change does this PR introduce?

Refactors some commands internals.

  • What is the current behavior?

cli/core commands are currently calling commands/core function without using rpc Requests structs.

  • What is the new behavior?

All cli/core commands now call commands/core functions using the rpc Requests structs.

Nope.

  • Other information:

We don't fix any issue with this PR but we'll avoid breaking the CLI and the gRPC interface alignment.

This adds more output to core list --format json but it doesn't break anything.

This change started out from a comment by @cmaglie on #1166, we should rebase that branch on top of this and another subsequent PR that will fix some legacy tests to verify that hardware loading works as expected.


See how to contribute

Copy link
Member

@cmaglie cmaglie left a comment

Choose a reason for hiding this comment

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

LGTM

@silvanocerza silvanocerza merged commit 8b5179d into master Feb 5, 2021
@silvanocerza silvanocerza deleted the scerza/fix-core-commands branch February 5, 2021 16:15
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

2 participants