Skip to content
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

Skip updating an existing issue when update_existing=false. #112

Merged
merged 2 commits into from
Sep 28, 2021

Conversation

dblock
Copy link
Contributor

@dblock dblock commented Sep 21, 2021

As far as I can see update_existing=garbage or update_existing=false works as update_existing=true today because Boolean doesn't parse but casts anything to true/false. With this change update_existing can be true, false and undefined.

  • When true, an existing issue is updated.
  • When false, an existing issue is not updated.
  • When undefined, a new issue is always created.

Closes #108.

Copy link
Owner

@JasonEtco JasonEtco left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks great @dblock, thanks so much for adding the tests and README updates. I left one minor comment about a tiny bit of code clarity, but otherwise this is good to go.

src/action.ts Outdated Show resolved Hide resolved
Comment on lines +12 to +21
let updateExisting: Boolean | null = null
if (tools.inputs.update_existing) {
if (tools.inputs.update_existing === 'true') {
updateExisting = true
} else if (tools.inputs.update_existing === 'false') {
updateExisting = false
} else {
tools.exit.failure(`Invalid value update_existing=${tools.inputs.update_existing}, must be one of true or false`)
}
}
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks great, thanks for adding the validation check (and the test) 🔥 ✨

@dblock
Copy link
Contributor Author

dblock commented Sep 28, 2021

Rebased, updated with the suggested change, and ready to merge.

Copy link
Owner

@JasonEtco JasonEtco left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Woohoo! Thanks for doing this 🎉

@JasonEtco JasonEtco merged commit f74c314 into JasonEtco:main Sep 28, 2021
@JasonEtco
Copy link
Owner

Published in https://github.com/JasonEtco/create-an-issue/releases/tag/v2.6.0! Thanks again for the contribution 🙌

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Allow for not touching existing issues
2 participants