Skip to content

Conversation

@skinny85
Copy link
Contributor

  1. Rename 'BuildProject' to 'Project'.
  2. Allow setting the physical name of a Project, to make it consistent with other L2s.
  3. Introduce a BuildImage class that makes it more convenient to specify the used Docker image.
  4. Introduce a convenience PipelineProject class for use in CodePipeline that defaults the source and artifacts fields.

Continued from #3


By submitting this pull request, I confirm that my contribution is made under
the terms of the beta license.

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.

Looks great!

const buildStage = new codepipeline.Stage(pipeline, 'Build');
new codebuildPipeline.PipelineBuildAction(buildStage, 'CodeBuild', {
project: project,
project: project
Copy link
Contributor

Choose a reason for hiding this comment

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

No need : project

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed.

// see the @aws-cdk/aws-codebuild module for more documentation on how to create CodeBuild Projects
const project = new codebuild.BuildProject( // ...
);
const project = new codebuild.Project(this, 'MyProject', {
Copy link
Contributor

Choose a reason for hiding this comment

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

Do you think there's value in walking through the layers here? Why would anyone work with this library if they don't want to use PipelineProject?

Copy link
Contributor

Choose a reason for hiding this comment

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

Perhaps you can switch the order of the narrative? Start with PipelineProject and then explain that under the hood it is just a sugar for Project

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The main draw to this module is the PipelineBuildAction (and soon the PipelineTestAction), PipelineProject is just a small convenience :). But sure, I've switched around the narrative to start with PipelineProject.

1. Rename 'BuildProject' to 'Project'.
2. Allow setting the physical name of a Project, to make it consistent with other L2s.
3. Introduce a BuildImage class that makes it more convenient to specify the used Docker image.
4. Introduce a convenience PipelineProject class for use in CodePipeline that defaults the source and artifacts fields.
@skinny85
Copy link
Contributor Author

Looks great!

Thanks :)

@skinny85
Copy link
Contributor Author

I would appreciate a merge if this looks good :)

@eladb eladb merged commit b8ebcbe into aws:master Jul 29, 2018
@skinny85 skinny85 deleted the feature/code-build-improvements2 branch July 30, 2018 16:52
@NGL321 NGL321 added the contribution/core This is a PR that came from AWS. label Sep 27, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

contribution/core This is a PR that came from AWS.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants