New addToPipeline methods for S3 Bucket and CodeBuild Project
#642
Conversation
|
|
||
| ```ts | ||
| // equivalent to the code above: | ||
| project.addBuildToPipeline(buildStage, 'CodeBuild', { |
There was a problem hiding this comment.
Why not just addToPipeline? It's a build project after all...
If this is because a build project can be added as a test action, this can be indicated in a prop
There was a problem hiding this comment.
You're correct Elad - the reason is that there will also be a test Action in the future (PipelineTestAction) that uses CodeBuild Projects.
My justification for separating them is that PipelineBuildAction and PipelineTestAction may actually have different construction properties (the test Action doesn't produce outputs, for example). Separation into addBuildToPipeline and addTestToPipeline gives us the flexibility to have these different props. If we have one addToPipeline method with a type: ProjectType.BUILD/.TEST property, we will forever be locked into them having the same properties.
Does this make sense?
There was a problem hiding this comment.
Actually, we simply can't express addToPipeline at all now - this method would have to return either the PipelineBuildAction or the PipelineTestAction type, and there is no way of expressing that without using generics, which JSII does not support.
There was a problem hiding this comment.
They could share a base class that has a disambiguator property. But that's not so great developer experience...
See #265 for details.
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license.