-
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
feat(codebuild): add support for setting the gitCloneDepth property on Project sources #1798
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 |
---|---|---|
|
@@ -71,17 +71,48 @@ export class NoSource extends BuildSource { | |
} | ||
} | ||
|
||
/** | ||
* The construction properties common to all build sources that are backed by Git. | ||
*/ | ||
export interface GitBuildSourceProps extends BuildSourceProps { | ||
/** | ||
* The depth of history to download. Minimum value is 0. | ||
* If this value is 0, greater than 25, or not provided, | ||
* then the full history is downloaded with each build of the project. | ||
*/ | ||
cloneDepth?: number; | ||
} | ||
|
||
/** | ||
* A common superclass of all build sources that are backed by Git. | ||
*/ | ||
export abstract class GitBuildSource extends BuildSource { | ||
private readonly cloneDepth?: number; | ||
|
||
protected constructor(props: GitBuildSourceProps) { | ||
super(props); | ||
|
||
this.cloneDepth = props.cloneDepth; | ||
} | ||
|
||
public toSourceJSON(): CfnProject.SourceProperty { | ||
const ret = super.toSourceJSON(); | ||
ret.gitCloneDepth = this.cloneDepth; | ||
return ret; | ||
} | ||
} | ||
|
||
/** | ||
* Construction properties for {@link CodeCommitSource}. | ||
*/ | ||
export interface CodeCommitSourceProps extends BuildSourceProps { | ||
export interface CodeCommitSourceProps extends GitBuildSourceProps { | ||
repository: codecommit.IRepository; | ||
} | ||
|
||
/** | ||
* CodeCommit Source definition for a CodeBuild project. | ||
*/ | ||
export class CodeCommitSource extends BuildSource { | ||
export class CodeCommitSource extends GitBuildSource { | ||
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. BTW, based on our discussions around #1743, this class should technically be in CodeCommit... 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. I think you're probably right, but that seems like a separate change. |
||
public readonly type: SourceType = SourceType.CodeCommit; | ||
private readonly repo: codecommit.IRepository; | ||
|
||
|
@@ -153,7 +184,7 @@ export class CodePipelineSource extends BuildSource { | |
/** | ||
* Construction properties for {@link GitHubSource} and {@link GitHubEnterpriseSource}. | ||
*/ | ||
export interface GitHubSourceProps extends BuildSourceProps { | ||
export interface GitHubSourceProps extends GitBuildSourceProps { | ||
/** | ||
* The GitHub account/user that owns the repo. | ||
* | ||
|
@@ -193,7 +224,7 @@ export interface GitHubSourceProps extends BuildSourceProps { | |
/** | ||
* GitHub Source definition for a CodeBuild project. | ||
*/ | ||
export class GitHubSource extends BuildSource { | ||
export class GitHubSource extends GitBuildSource { | ||
public readonly type: SourceType = SourceType.GitHub; | ||
private readonly httpsCloneUrl: string; | ||
private readonly oauthToken: cdk.Secret; | ||
|
@@ -228,7 +259,7 @@ export class GitHubSource extends BuildSource { | |
/** | ||
* Construction properties for {@link GitHubEnterpriseSource}. | ||
*/ | ||
export interface GitHubEnterpriseSourceProps extends BuildSourceProps { | ||
export interface GitHubEnterpriseSourceProps extends GitBuildSourceProps { | ||
/** | ||
* The HTTPS URL of the repository in your GitHub Enterprise installation. | ||
*/ | ||
|
@@ -250,8 +281,8 @@ export interface GitHubEnterpriseSourceProps extends BuildSourceProps { | |
/** | ||
* GitHub Enterprise Source definition for a CodeBuild project. | ||
*/ | ||
export class GitHubEnterpriseSource extends BuildSource { | ||
public readonly type: SourceType = SourceType.GitHubEnterPrise; | ||
export class GitHubEnterpriseSource extends GitBuildSource { | ||
public readonly type: SourceType = SourceType.GitHubEnterprise; | ||
private readonly httpsCloneUrl: string; | ||
private readonly oauthToken: cdk.Secret; | ||
private readonly ignoreSslErrors?: boolean; | ||
|
@@ -275,7 +306,7 @@ export class GitHubEnterpriseSource extends BuildSource { | |
/** | ||
* Construction properties for {@link BitBucketSource}. | ||
*/ | ||
export interface BitBucketSourceProps extends BuildSourceProps { | ||
export interface BitBucketSourceProps extends GitBuildSourceProps { | ||
/** | ||
* The BitBucket account/user that owns the repo. | ||
* | ||
|
@@ -294,7 +325,7 @@ export interface BitBucketSourceProps extends BuildSourceProps { | |
/** | ||
* BitBucket Source definition for a CodeBuild project. | ||
*/ | ||
export class BitBucketSource extends BuildSource { | ||
export class BitBucketSource extends GitBuildSource { | ||
public readonly type: SourceType = SourceType.BitBucket; | ||
private readonly httpsCloneUrl: any; | ||
|
||
|
@@ -318,7 +349,7 @@ export enum SourceType { | |
CodeCommit = 'CODECOMMIT', | ||
CodePipeline = 'CODEPIPELINE', | ||
GitHub = 'GITHUB', | ||
GitHubEnterPrise = 'GITHUB_ENTERPRISE', | ||
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. LOLx |
||
GitHubEnterprise = 'GITHUB_ENTERPRISE', | ||
BitBucket = 'BITBUCKET', | ||
S3 = 'S3', | ||
} |
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.
I think this is an example of over-modeling. Now what happens if we have another Git source that's not in the codebuild package, then what...
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.
But I was so proud of how I DRYied it up... 😭
What alternative would you propose here Elad?
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.
Just duplicate. Sometimes it's too DRY
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.
But I am okay with this too :-D
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.
I can't. I can't bear myself to copy & paste
cloneDepth
after I've so beautifully applied inheritance and the Decorator pattern here. My object-oriented programming education forbids me.I'll leave this as is for now, feel free to open a separate PR changing this behind my back 😃