Cosmos-sdk: CLI Context Renaming

Created on 10 Dec 2018  ·  11Comments  ·  Source: cosmos/cosmos-sdk

Summary

CLIContext has grown out of a refactor of the client package (rightfully so) and has become a powerhouse in facilitating much of the client functionality. However, I'm seeing it being used in REST components and other areas where a "CLI" is not really involved.

Proposal

Both the CLI and REST I see as client interfaces. I simply propose we rename CLIContext to client.Context.

/cc @jackzampolin @alessio @fedekunze


For Admin Use

  • [ ] Not duplicate issue
  • [ ] Appropriate labels applied
  • [ ] Appropriate contributors tagged
  • [ ] Contributor assigned/self-assigned
CLI REST dev-ux good first issue help wanted proposal-accepted

Most helpful comment

I can take this up, it will also help us organize keys cli tests as @alessio suggested

All 11 comments

++

Proposal Accepted!

I think we should do this sooner than later cc @alexanderbez @aaronc.

Sure 👍 . Why not just client.Context?

Absolutely!

Why not just client.Context

YES! please

I can take this up, it will also help us organize keys cli tests as @alessio suggested

Thank you so much @sahith-narahari 🙏

This is summary of discussion from discord
@alexanderbez proposed just renaming the CliContext to Context(will be imported as context.Context) but this is confusing as it collides with stdlib's Context which will be used once clients use gRPC

@aaronc suggested that the top level client package is for things that are likely to be imported a lot and CLIContext is currently imported a lot. so either we move some stuff that's in client/ elsewhere and/or we package client/ a top-level packages that aliases inner packages like the modules do

I would like to reiterate, grouping and packages should be done by context/domain and not by expected usage. However, clobbering with the stdlib context is a valid concern and may warrant moving Context types and logic to the root client package. As long as this is a painless effort, I'm onboard 👍

+1 for moving CLIContext into client

Was this page helpful?
0 / 5 - 0 ratings