Support Disabling Source Type Override Parameters #64
Conversation
|
Thank you for working on this and submitting this pull request. We want this so we can use s3 as a source instead of GitHub. However, when testing your changes, s3 as a source fails with the following error: |
| @@ -13,6 +13,9 @@ inputs: | |||
| env-vars-for-codebuild: | |||
| description: 'Comma separated list of environment variables to send to CodeBuild' | |||
| required: false | |||
| disable-source-override: | |||
| description: 'Set to `true` if you want do disable source repo override' | |||
|
May I ask why this is not merged ? I need this feature very much in my project |
|
Is it going to be merged? I just want GitHub actions to trigger CodeBuild to run build commands in VPC. It doesn't need a source of GITHUB |
|
@smailc I actually forked and tweaked it to do this https://github.com/mostmentor/aws-codebuild-run-build |
There was a problem hiding this comment.
@khola - Thank you so much for making this change.
Apology for the delayed review. If you are still interested in wrapping up and merging this pull request, please address the comment and resolve the conflicts.
I am ready to approve and release once the comments have been addressed
| @@ -110,6 +110,8 @@ async function waitForBuildEndTime(sdk, { id, logs }, nextToken) { | |||
|
|
|||
| function githubInputs() { | |||
| const projectName = core.getInput("project-name", { required: true }); | |||
| const disableSourceOverride = | |||
There was a problem hiding this comment.
nit - For consistency, could you either update this new line to use core.getInput("disable-source-override", {required: false}) === "true", or remove all other { required: false }s from githubInputs()?
| @@ -94,6 +94,21 @@ describe("githubInputs", () => { | |||
| .and.to.deep.equal(["one", "two", "three", "four"]); | |||
| }); | |||
|
|
|||
| it("skips override when parameter is set to true", () => { | |||
There was a problem hiding this comment.
-
could you also add a assertion in build basic parameters for codeBuild.startBuild that expect(test).to.haveOwnProperty("disableSourceOverride").and.to.equal(false);
-
the test case name is a bit confusing. should we just called it
"can process disable-source-override"
| repo: "repo", | ||
| disableSourceOverride: true, | ||
| }); | ||
| expect(test).to.not.haveOwnProperty("sourceTypeOverride"); |
There was a problem hiding this comment.
expect(test).to.not.haveOwnProperty("sourceLocationOverride");
|
close this in favor of #102 |
Issue #, if available:
#57
Description of changes:
Added additional input to allow for disabling the parameters
sourceTypeOverrideandsourceLocationOverride. Test cases added.By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
Check any applicable: