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

Rename simapp #13872

Closed
okwme opened this issue Nov 15, 2022 · 9 comments
Closed

Rename simapp #13872

okwme opened this issue Nov 15, 2022 · 9 comments

Comments

@okwme
Copy link
Contributor

okwme commented Nov 15, 2022

Summary

some chains are importing simapp from cosmos-sdk when they should be importing their own app (for ex cosmos/gaia#1883). i think this comes from a misunderstanding of the meaning of "simapp"—as in the simulations that are run on every chain. Rather simapp is more of a "sample app" so i suggest renaming it to "sampleApp" or something similar.

Problem Definition

Proposal

@alexanderbez
Copy link
Contributor

alexanderbez commented Nov 15, 2022

The original intent was for an app to be used in the SDK's simulation testing framework, hence "SimApp". But totally down to rename it :)

That being said, post 0.47 world, simapp will be completely internal (it already is in main). So apps/repo's won't even be able to import it even if they wanted to -- no foot gun.

So sort of a moot issue at this point?

@okwme
Copy link
Contributor Author

okwme commented Nov 15, 2022

ja seems the issue will go away naturally 👍

@okwme okwme closed this as completed Nov 15, 2022
@gjermundgaraba
Copy link
Contributor

Can I offer a counterpoint to this? Many people go into the SDK repo to figure out how to use the SDK, and the simapp name is still confusing and leads to misunderstandings. A better name, even if fully internal, seems like a good idea to me.

@tac0turtle
Copy link
Member

do you mean moving simapp to internal?

@gjermundgaraba
Copy link
Contributor

I mean renaming simapp to make it clear what it is :)

@julienrbrt
Copy link
Member

julienrbrt commented Feb 20, 2023

If someone reads the docs, it is the first line in the README. I am confused why it is confusing 😅
A person onboarding should read the docs.
image

Imho, as it is the app we use for actually testing the SDK (from e2e to simulations) and that it is a chain, Sim App makes sense. At least it has never been confusing when I onboarded because it was explained in the docs.

I think (I don't know) before people imported simapp to reduce boilerplate and because there was not a clear separation from the app and the testing utilities (which could then be confusing I imagine). But, as bez mentioned above, is resolved in v0.47+.

@gjermundgaraba
Copy link
Contributor

gjermundgaraba commented Feb 20, 2023

Perhaps not a huge problem, but I think the name gives the wrong impression; of being an app for simulation. Yes, it is also used to run simulation tests for the SDK, but for most people using the SDK repo it's a demo app to see how to implement, wire up and do different things. It's a reference implementation for all intents and purposes (outside of the SDK's internal usage of it).

I guess it's not a big deal, but even if a name is documented to mean something else than what is seems like, it doesn't make it any better :) <- in my opinion.

Edit: another issue I realized that bugged me a while back was that the names also muddles up the sim name when used for simulation testing. The testutils sims package is a good example, because it gets used a lot in the simapp, but is actually a testutil package for simulation testing (but easily confused with testing utility for simapp). I think the fact that sim/sims/simapp is used in multiple places and mean different things just add to my feeling that the SimApp should be better named.

@robert-zaremba
Copy link
Collaborator

  1. everyone in cosmos ecosystem knows simapp
  2. that being said, I 100% agree that there could be a better name. Since no external application should import simapp as a package, then there should not be a problem to rename that.

@gjermundgaraba
Copy link
Contributor

everyone in the cosmos ecosystem knows simapp

That may be true for veterans in the ecosystem, but I am not sure this is true for newer additions. And it is not for people who already know this that I am suggesting to change the name, but for new people coming in and needing clarification - just like I did.

Perhaps it is indeed inconsequential, but I'm a stickler for good naming because it requires less documentation and makes thing smoother :)

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

No branches or pull requests

6 participants