-
Notifications
You must be signed in to change notification settings - Fork 140
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
Support Disabling Source Type Override Parameters, including Source Version #102
Conversation
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.
Thank you so much for making this enhancement.
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
code-build.js
Outdated
@@ -152,6 +152,8 @@ async function waitForBuildEndTime( | |||
|
|||
function githubInputs() { | |||
const projectName = core.getInput("project-name", { required: true }); | |||
const disableSourceOverride = | |||
core.getInput("disable-source-override") === "true"; |
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.
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()?
README.md
Outdated
@@ -31,6 +31,8 @@ The only required input is `project-name`. | |||
1. **image-override** (optional) : | |||
The name of an image for this build that overrides the one specified | |||
in the build project. | |||
1. **disable-source-override** (optional) : | |||
Set to `true` if you want to disable providing `sourceTypeOverride` and `sourceLocationOverride` to CodeBuild. |
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.
and sourceVersion
# Conflicts: # action.yml # code-build.js # dist/index.js
@marcus-bcl could you resolve the conflict, so I can merge this PR ? |
# Conflicts: # dist/index.js
Apologies I didn't see that your approval, I've resolved the conflict now - feel free to merge it. Thanks! |
Looks like this PR breaks existing usages @taoyong-ty.
|
Our CI too is failing with the above |
Issue #, if available:
#57
Description of changes:
This PR is an extension of #64 to ensure all three source override parameters are disabled. It also fixes merge conflicts and npm audit issues.
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
Check any applicable: