-
Notifications
You must be signed in to change notification settings - Fork 3.9k
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
[BREAKING] Pass the Pipeline to Stage through props instead of parent #568
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -6,34 +6,45 @@ import { cloudformation } from './codepipeline.generated'; | |
import { Pipeline } from './pipeline'; | ||
|
||
/** | ||
* A stage in a pipeline. Stages are added to a pipeline by constructing a Stage with | ||
* the pipeline as the first argument to the constructor. | ||
* The construction properties for {@link Stage}. | ||
*/ | ||
export interface StageProps { | ||
/** | ||
* The Pipeline to add the newly created Stage to. | ||
*/ | ||
pipeline: Pipeline; | ||
} | ||
|
||
/** | ||
* A Stage in a Pipeline. | ||
* Stages are added to a Pipeline by constructing a new Stage, | ||
* and passing the Pipeline it belongs to through the {@link StageProps#pipeline} attribute. | ||
* | ||
* @example | ||
* // add a stage to a pipeline | ||
* new Stage(pipeline, 'MyStage'); | ||
* // add a Stage to a Pipeline | ||
* new Stage(this, 'MyStage', { | ||
* pipeline: myPipeline, | ||
* }); | ||
*/ | ||
export class Stage extends cdk.Construct implements actions.IStage { | ||
/** | ||
* The Pipeline this stage is a member of | ||
* The Pipeline this Stage is a part of. | ||
*/ | ||
public readonly pipeline: Pipeline; | ||
public readonly name: string; | ||
|
||
private readonly _actions = new Array<actions.Action>(); | ||
|
||
/** | ||
* Append a new stage to the pipeline | ||
* | ||
* Only a Pipeline can be passed in as a parent because stages should | ||
* always be attached to a pipeline. It's illogical to construct a Stage | ||
* with any other parent. | ||
* Create a new Stage. | ||
*/ | ||
constructor(parent: Pipeline, name: string) { | ||
constructor(parent: cdk.Construct, name: string, props: StageProps) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. might make sense to allow people to customize the stage name such that it's not the construct id (which is a sensible default) There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes! This is actually part of a bigger discussion about Stage / Action naming that I want to have with you guys. |
||
super(parent, name); | ||
this.name = name; | ||
this.pipeline = parent; | ||
this.pipeline = props.pipeline; | ||
actions.validateName('Stage', name); | ||
|
||
this.pipeline._addStage(this); | ||
} | ||
|
||
/** | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
document, tell users that they don't need to use it directly and what they should be using
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍🏻