Closed shawntabrizi closed 3 years ago
I really like that more of the work is pushed back to git and I really like that the git history is kept in tact. These are both positive changes.
I miss the ability to specify the name of the module when using substrate-module-new
but it sounds like you're addressing that by having a script in the module template that will allow me to give the new module a meaningful name, right? If so I'm comfortable with that :+1:
The biggest outstanding question/concern I have is how the substrate-package
branches will be managed in each template repository. Cloning from a simple branch called substrate-package
which move as new features are included in the package is quite dangerous. Consider this scenario
substrate-node-new
, substrate-module-new
, and substrate-ui-new
to get started. They all pull from known-compatible branches.substrate-package
branches of several of these templates.substrate-module-new
command as before but now gets a different module template; potentially one that isn't even compatible with the node template she has been working on.I think a nicer solution would be to have each script have an option parameter for a git reference (branch, tag, commit, etc) to clone from. If a user omits that flag then the default behavior is to clone from the substrate-package
branch just as it does now. The script should also tell the user what reference they can use in the future to get the exact same version of the template they just got.
So in the above scenario Alice would run substrate-module-new first-module
the first time and see text like, "Creating a new module template from tag package-02-Sept-2019
. Then in step three she would run substrate-module-new second-module package-02-Sept-2019
to ensure she gets the same exact template she got the first time.
@JoshOrndorff A fair concern.
Adding a git commit makes sense to me as a second (optional) parameter.
Two things I do want to note:
I do think that these substrate-up scripts will ultimately only be used for legacy/niche cases. I would much prefer our documentation and tutorials to teach people to literally clone from the master
/substrate-package
branch by running the git commands themselves. This ensures that our docs work for everyone, not just those with bash.
I also hope that we will soon be able to pin all of these items to the v2.0 branch, which means there should never be a situation where things are incompatible as you describe.
I have not thought that hard about the renaming story, since, well, I think it is a little soft. I would literally prefer we tell the user to use their text editor to do find/replace rather than have some script automatically do it for them. It has also been suggested that we can use cargo generate
. But I don't know if that will mess up our template being used out of the box.
I think we should look into the renaming thing next, but as the PR points out, this can be handled on the individual repository level, which again is nice to separate from these scripts because it may be updated in the future.
I think we should still keep some handholding messages such as how to push the repo to a new git repo and detect if yarn is installed. It should also rename remote from origin to upstream, checkout new master branch and ensure it is not tracking upstream master. You don't want people accidentally trying to push template repo. not that it will success, but still can be confusing. I still find the rename functionality useful. It makes the generated project more personalised, not just a generic template project.
@xlc I find the rename thing the absolute worst part of the script. If someone really feels "special" that the project is renamed, they can do it themselves.
Having generic find/replace things in a script is totally set up to fail at some point in some way we wont expect.
I agree about the upstream thing, what is the best way to do that?
How about we just get rid of these scripts all together?
I think that this scripts are a great way to start developing on Substrate. I also see that maintenance will get more complex every time we add more scripts. What will happen if we deprecate a script? We'll need to find the best way to delete it because we are not using a package manager.
I think that it will be better if the scripts become one single tool like NVM instead of installing many scripts and loosing control of what the user has installed. It will be more complex to create a NVM equivalent but it will give us more control and we can handle versioning in a better way.
There is not much difference in updating or deleting a script.
Users who have installed the script on their machine will not get updates to the script unless they manually reinstall the script on their machine.
This is the fundamental problem with any of these scripts is that they do not scale well over time when we want to tweak the behavior.
This is why I am a fan of either:
making the scripts just be a simple pointer to git clone
and in our docs never really using the scripts (just supporting backwards compatibility)
Getting rid of the scripts all together and really just live a better life
Here is a proposed change to all the scripts, hopefully making them less magic, and depending more on Git.
Each script depends on a specific(not needed, we assume all git repos in Substrate Developer Hub are compatible)substrate-package
branch, which will move at a slower pace thanmaster
, but guarantees compatibility between other templates with the samesubstrate-package
branch<NAME>
which is used to determine the folder name.Open questions:
init.sh
or similar script after cloning?init
script which does all the stuff that was done before with renamingCC @xlc