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

aragon init prepare template for environments #371

Merged
merged 5 commits into from
Feb 21, 2019
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Copy link
Contributor

Choose a reason for hiding this comment

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

Just noticed this, but it seems unnecessary to run documentation lint on the tests 😄

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'll include this change in #359

"extract-roles": "node scripts/extract-roles"
},
"repository": {
Expand Down Expand Up @@ -116,7 +116,7 @@
"@babel/register"
],
"files": [
"test/**/*.js"
"test/commands/**/*.js"
]
},
"aragon": {
Expand Down
31 changes: 12 additions & 19 deletions src/lib/init/prepare-template.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,25 +5,18 @@ export async function prepareTemplate(basename, appName) {
const arappPath = path.resolve(basename, 'arapp.json')
const arapp = await fs.readJson(arappPath)

// TODO remove once the old arapp.json is no longer supported
if (!arapp.environments) {
arapp.environments = {}
}

const props = {
network: 'development',
appName: appName,
}

if (arapp.environments.default) {
Object.assign(arapp.environments.default, props)
} else {
arapp.environments.default = props
}

// remove old arapp.json props
delete arapp.appName
delete arapp.version
const defaultEnv = arapp.environments.default
const stagingEnv = arapp.environments.staging
const productionEnv = arapp.environments.production

defaultEnv.appName = appName
Object.assign(arapp.environments.default, defaultEnv)

stagingEnv.appName = stagingEnv.appName.replace(/^app/, appName)
Copy link
Contributor

Choose a reason for hiding this comment

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

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]
   //...
}

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, you are right. This is better

productionEnv.appName = productionEnv.appName.replace(/^app/, appName)

Object.assign(arapp.environments.staging, stagingEnv)
Object.assign(arapp.environments.production, productionEnv)

const gitFolderPath = path.resolve(basename, '.git')
const licensePath = path.resolve(basename, 'LICENSE')
Expand Down
18 changes: 15 additions & 3 deletions test/commands/init.js
Original file line number Diff line number Diff line change
Expand Up @@ -27,14 +27,24 @@ test('prepare project template', async t => {
const arappPath = `${projectPath}/arapp.json`
const packageJsonPath = `${projectPath}/package.json`
const licensePath = `${projectPath}/LICENSE`
const appName = 'TestApp'
const appName = 'aragon-app.aragonpm.eth'

await fs.ensureDir(repoPath)
await fs.ensureFile(arappPath)
await fs.ensureFile(packageJsonPath)
await fs.ensureFile(licensePath)
await fs.writeJson(arappPath, {
appName: 'boilerplate-placeholder',
environments: {
default: {
appName: 'app.aragonpm.eth',
},
staging: {
appName: 'app.open.aragonpm.eth',
},
production: {
appName: 'app.open.aragonpm.eth',
},
},
})
await fs.writeJson(packageJsonPath, {
license: 'MIT',
Expand All @@ -48,5 +58,7 @@ test('prepare project template', async t => {
t.falsy(await fs.pathExists(repoPath))
t.is(undefined, packageJson.license)
t.falsy(fs.pathExistsSync(licensePath))
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)
Copy link
Contributor

Choose a reason for hiding this comment

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

Looks like appName for staging and prod will be aragon-app.aragonpm.eth.open.aragonpm.eth

})