-
Notifications
You must be signed in to change notification settings - Fork 316
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
chore: remove sims #697
chore: remove sims #697
Conversation
resolves celestiaorg#693
Will close in favor of #692 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
do we also need to remove something from the root makefile?
I think we can reopen this since it can be isolated.
Yes. Likely https://github.com/celestiaorg/celestia-app/blob/main/Makefile#L76-L81 |
yeah, exactly, will approve after that is removed and the makefile works |
include contrib/devtools/Makefile | ||
include contrib/devtools/sims.mk | ||
|
||
test-all: check test-race test-cover |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
check
was removed b/c it is not defined in any of the Makefiles
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍
I noticed that we no longer need a Makefile `PACKAGES` variable because this repo doesn't contain a `/simulation` directory. We removed sims from workflows in #697 but there wasn't a `/simulation` directory then either. This makes the `make test` command easier to copy + paste (if for instance you want to add a trailing `-count=1` flag to disable the cache) Note: cosmos-sdk uses PACKAGE variables [here](https://github.com/cosmos/cosmos-sdk/blob/main/Makefile#L3-L4) but celestia-app doesn't appear to need this extra complexity
I noticed that we no longer need a Makefile `PACKAGES` variable because this repo doesn't contain a `/simulation` directory. We removed sims from workflows in celestiaorg#697 but there wasn't a `/simulation` directory then either. This makes the `make test` command easier to copy + paste (if for instance you want to add a trailing `-count=1` flag to disable the cache) Note: cosmos-sdk uses PACKAGE variables [here](https://github.com/cosmos/cosmos-sdk/blob/main/Makefile#L3-L4) but celestia-app doesn't appear to need this extra complexity
I noticed that we no longer need a Makefile `PACKAGES` variable because this repo doesn't contain a `/simulation` directory. We removed sims from workflows in celestiaorg/celestia-app#697 but there wasn't a `/simulation` directory then either. This makes the `make test` command easier to copy + paste (if for instance you want to add a trailing `-count=1` flag to disable the cache) Note: cosmos-sdk uses PACKAGE variables [here](https://github.com/cosmos/cosmos-sdk/blob/main/Makefile#L3-L4) but celestia-app doesn't appear to need this extra complexity
Resolves #693