aragon init prepare template for environments #371
Conversation
test/commands/init.js
Outdated
| t.is(appName, project.environments.default.appName) | ||
| t.is(`${appName}`, project.environments.default.appName) | ||
| t.is(`${appName}.open.aragonpm.eth`, project.environments.staging.appName) | ||
| t.is(`${appName}.open.aragonpm.eth`, project.environments.production.appName) |
There was a problem hiding this comment.
Looks like appName for staging and prod will be aragon-app.aragonpm.eth.open.aragonpm.eth
src/lib/init/prepare-template.js
Outdated
| defaultEnv.appName = appName | ||
| Object.assign(arapp.environments.default, defaultEnv) | ||
|
|
||
| stagingEnv.appName = stagingEnv.appName.replace(/^app/, appName) |
There was a problem hiding this comment.
To avoid stagingEnv.appName = 'aragon-app.aragonpm.eth.open.aragonpm.eth'
we could have:
- stagingEnv.appName = stagingEnv.appName.replace(/^app/, appName)
+ stagingEnv.appName = stagingEnv.appName.replace(/^app/, basename)this will work for the CLI, but break the test, because in the test we are passing a path: ./.tmp/aragon-app, and not the basename.
I think what we can do instead is some parsing in prepareTemplate (get the basename from appName) and rename the param to path:
function prepareTemplate(path, appName) {
const basename = name.split('.')[0]
//...
}There was a problem hiding this comment.
Yes, you are right. This is better
| @@ -11,7 +11,7 @@ | |||
| "build": "npm run extract-roles && babel -d dist src --copy-files", | |||
| "prepare": "npm run build", | |||
| "test": "ava", | |||
| "lint": "eslint src test && documentation lint src test", | |||
| "lint": "eslint src test/commands && documentation lint src test/commands", | |||
There was a problem hiding this comment.
Just noticed this, but it seems unnecessary to run documentation lint on the tests 😄
There was a problem hiding this comment.
I'll include this change in #359
| const licensePath = path.resolve(basename, 'LICENSE') | ||
|
|
||
| const packageJsonPath = path.resolve(basename, 'package.json') | ||
| const packageJsonPath = path.resolve(dir, 'package.json') | ||
| const packageJson = await fs.readJson(packageJsonPath) | ||
| delete packageJson.license |
There was a problem hiding this comment.
Just curious, should we move this to also be in create-aragon-app (really really want to start pushing that!)?
There was a problem hiding this comment.
Yes! I already made the same changes in create-aragon-app. Seems its never the perfect time to merge the monorepo PR to finally have create-aragon-app in aragonCLI 😅Maybe after the beta release?
There was a problem hiding this comment.
After the next release (5.4) would be a great time!
No description provided.