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

Move all binaries to examples #242

Merged
merged 9 commits into from
Sep 4, 2017
Merged

Conversation

ethanfrey
Copy link
Contributor

Now with examples/eyes examples/counter and examples/basecoin that each are self-contained

All cli tests and installation belong to an example and are called from the main Makefile.

I think there are some dependencies between the examples (they import basecoin/cmd/commands....), which should be removed to make this cleaner (no dependencies on examples), and maybe some more cleanup.

. $DIR/shunit2

# TODO: how to handle this if we are not in the same directory
CLI_DIR=${DIR}/../../../../tests/cli
Copy link
Contributor

Choose a reason for hiding this comment

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

The $DIR command navigates to a directory relative to THIS bash file... So this shouldn't be an issue, alternatively (maybe more clearly) we could just make $DIR a function of the $GOPATH

Copy link
Contributor

Choose a reason for hiding this comment

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

CLI_DIR=$GOPATH/src/github.com/cosmos/cosmos-sdk/tests/cli

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Looks better to me

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Maybe in a different repo, we would refer to the cosmos-sdk in vendor dir

./tests/cli/ibc.sh

test_tutorial: docs/guide/shunit2
# wget "https://raw.githubusercontent.com/kward/shunit2/master/source/2.1/src/shunit2"
Copy link
Contributor

Choose a reason for hiding this comment

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

wait how are we retrieving wget now?

@rigelrozanski
Copy link
Contributor

Okay generally, I'm a fan of the idea of the this consolidation, however I think there is to much useful code in the examples dir. I'd like to see the example Directory be disposable to the repo, aka nothing should be build to depend on its features, it should exclusively depend on the SDK. In this sense it's really examples only not automation which may want to get used by another repo. for starters init.go which is a part of the basecoin/cmd/basecoin/commands (https://github.com/cosmos/cosmos-sdk/tree/feature/move_to_examples/examples/basecoin/cmd/basecoin/commands) is really general purpose, especially after PR #243. All of this "general-purpose" command related stuff should probably not be in the example repo, but in a command helper repo somewhere. I think an easy way to determine this is may to just look at what kinds of stuff the counter app is importing from basecoin... all that stuff

@ethanfrey
Copy link
Contributor Author

I agree.

I think this consolidation helps find pieces of one app that are general purpose and we have to refactor them out of the examples cleanly.

Cmd basecoin commands is a good example
As is the common.sh for the tests
Any more?

We don't use wget anymore but I added the file directly to the repo. Jae said it was a security issue to wget on every test. Who knows what is in that repo.

@rigelrozanski
Copy link
Contributor

oh I see, okay yeah I guess that's fine, would have done the same, just wasn't sure about straight up copying a repo - looks good.
To answer your question... that might be it actually, basecoin/cmd/basecoin/commands really only

@rigelrozanski
Copy link
Contributor

Just noticed we have been using a bit of ambiguity around the language cli sometimes this is referring to client and sometimes it's referring to command line interface not sure if there is a good way which we can clean this up

Another comment, I want to be able to bash test modules... right now it looks like maybe the only way to do that is to include those tests in the basecoin cli tests, which is less than ideal because it's core tests in the example dir... ideaS?

@ethanfrey ethanfrey changed the title WIP: Move all binaries to examples Move all binaries to examples Sep 4, 2017
@ethanfrey ethanfrey merged commit c94500f into develop Sep 4, 2017
@ethanfrey ethanfrey deleted the feature/move_to_examples branch September 4, 2017 17:11
alexanderbez pushed a commit that referenced this pull request May 26, 2022
cref: #9812

## Description

This PR adds a CLI for querying all module accounts, `module-accounts`, in the auth module. 
Using this command would display all the account information, including human readable name of the module, module account address, account number, permissions, etc. 

This command would be especially useful for developers using cosmos-sdk as there are lots of instances where a developer would have to look into the module account, but only to do so by getting the module account address after getting the module name, hashing it and bech32fying it in the status quo. By using this command, users would be able to get a quick look on the list of all module accounts' information.


---

### 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](https://github.com/commitizen/conventional-commit-types/blob/v3.0.0/index.json) in the PR title
- [ ] added `!` to the type prefix if API or client breaking change
- [ ] targeted the correct branch (see [PR Targeting](https://github.com/cosmos/cosmos-sdk/blob/master/CONTRIBUTING.md#pr-targeting))
- [ ] provided a link to the relevant issue or specification
- [ ] followed the guidelines for [building modules](https://github.com/cosmos/cosmos-sdk/blob/master/docs/building-modules)
- [ ] included the necessary unit and integration [tests](https://github.com/cosmos/cosmos-sdk/blob/master/CONTRIBUTING.md#testing)
- [ ] added a changelog entry to `CHANGELOG.md`
- [ ] included comments for [documenting Go code](https://blog.golang.org/godoc)
- [ ] 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...

- [x] confirmed the correct [type prefix](https://github.com/commitizen/conventional-commit-types/blob/v3.0.0/index.json) in the PR title
- [x] 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
- [x] reviewed tests and test coverage
- [x] manually tested (if applicable)
yihuang added a commit to yihuang/cosmos-sdk that referenced this pull request Apr 1, 2024
Solution:
- init cachestore on cachestore lazily.
- cleanup some unused stuff.

Update store/CHANGELOG.md

Signed-off-by: yihuang <[email protected]>
dudong2 pushed a commit to b-harvest/cosmos-sdk that referenced this pull request Oct 17, 2024
Solution:
- init cachestore on cachestore lazily.
- cleanup some unused stuff.

Update store/CHANGELOG.md

Signed-off-by: yihuang <[email protected]>
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.

2 participants