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

feat(cli): run cdk from inner directories #5772

Merged
merged 11 commits into from
Jan 13, 2020

Conversation

iliapolo
Copy link
Contributor

@iliapolo iliapolo commented Jan 13, 2020

Added ability to run cdk commands from inner directories (inside the project root where cdk.json is located).

The cdk.js will now lookup the project directory by traversing to parent directories until it finds cdk.json.

Closes #5691


By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license

@iliapolo iliapolo force-pushed the epolon/cdk-from-inner-directories branch from d000945 to aee3033 Compare January 13, 2020 12:30
@iliapolo iliapolo force-pushed the epolon/cdk-from-inner-directories branch from aee3033 to aaa3b30 Compare January 13, 2020 12:33
Comment on lines 396 to 419
async function lookupProjectRoot(): Promise<string | undefined> {

async function ascend(directoryParts: string[]): Promise<string | undefined> {

if (directoryParts.length === 0) {
return undefined;
}

const directory = directoryParts.join(path.sep);

const filePath = path.join(directory, PROJECT_CONFIG);

if (await fs.pathExists(filePath)) {
return directory;
}

return ascend(directoryParts.slice(0, -1));

}

return ascend(path.resolve(process.cwd()).split(path.sep));

}

Copy link
Contributor

Choose a reason for hiding this comment

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

I guess I'd just use path.dirname instead of parsing the path:

async function ascend(dir: string) {
  if (dir === path.sep) { return undefined; }
  // ...
  return ascend(path.dirname(dir));
}

yarn.lock Outdated
@@ -1454,20 +1454,13 @@
resolved "https://registry.yarnpkg.com/@types/yargs-parser/-/yargs-parser-13.1.0.tgz#c563aa192f39350a1d18da36c5a8da382bbd8228"
Copy link
Contributor

Choose a reason for hiding this comment

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

revert yarn.lock

@aws-cdk-automation
Copy link
Collaborator

AWS CodeBuild CI Report

  • Result: SUCCEEDED
  • Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

@aws-cdk-automation
Copy link
Collaborator

AWS CodeBuild CI Report

  • Result: SUCCEEDED
  • Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

@aws-cdk-automation
Copy link
Collaborator

AWS CodeBuild CI Report

  • Result: SUCCEEDED
  • Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

@aws-cdk-automation
Copy link
Collaborator

AWS CodeBuild CI Report

  • Result: SUCCEEDED
  • Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

@aws-cdk-automation
Copy link
Collaborator

AWS CodeBuild CI Report

  • Result: SUCCEEDED
  • Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

@iliapolo iliapolo marked this pull request as ready for review January 13, 2020 13:33
@iliapolo
Copy link
Contributor Author

@eladb Have another look?

Copy link
Contributor

@eladb eladb left a comment

Choose a reason for hiding this comment

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

Nice!

Comment on lines 8 to 16

function cleanup() {
popd
set -e
}

trap cleanup EXIT INT

pushd ..
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't think the cleanup pushd/popd is needed. This script is being executed as a child process.

@aws-cdk-automation
Copy link
Collaborator

AWS CodeBuild CI Report

  • Result: SUCCEEDED
  • Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

eladb
eladb previously requested changes Jan 13, 2020
Comment on lines 9 to 12
pushd ..
set +e
output="$(cdk diff ${STACK_NAME_PREFIX}-test-1 2>&1)"
set -e
Copy link
Contributor

Choose a reason for hiding this comment

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

move to a temp directory instead. you don't know what's going on in ".."

@mergify mergify bot dismissed eladb’s stale review January 13, 2020 13:51

Pull request has been modified.

@aws-cdk-automation
Copy link
Collaborator

AWS CodeBuild CI Report

  • Result: SUCCEEDED
  • Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

@aws-cdk-automation
Copy link
Collaborator

AWS CodeBuild CI Report

  • Result: SUCCEEDED
  • Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

@aws-cdk-automation
Copy link
Collaborator

AWS CodeBuild CI Report

  • Result: SUCCEEDED
  • Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

@aws-cdk-automation
Copy link
Collaborator

AWS CodeBuild CI Report

  • Result: SUCCEEDED
  • Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

@mergify
Copy link
Contributor

mergify bot commented Jan 13, 2020

Thank you for contributing! Your pull request is now being automatically merged.

@aws-cdk-automation
Copy link
Collaborator

AWS CodeBuild CI Report

  • Result: SUCCEEDED
  • Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

@mergify
Copy link
Contributor

mergify bot commented Jan 13, 2020

Thank you for contributing! Your pull request is now being automatically merged.

@mergify mergify bot merged commit a54630d into master Jan 13, 2020
@mergify mergify bot deleted the epolon/cdk-from-inner-directories branch January 13, 2020 15:27
iliapolo added a commit that referenced this pull request Jan 16, 2020
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

Successfully merging this pull request may close these issues.

cli: support running cdk from inner directories
3 participants