ElayGelbart / Heroku-Auto-Deployment

MIT License
23 stars 10 forks source link

Fix useDocker input parsing #10

Closed addianto closed 2 years ago

addianto commented 2 years ago

Hello! First of all, I'm sorry if I did not create an issue before submitting this pull request (PR). However, since the issue and the possible fix is very straightforward, I just submitted the PR along with my explanation.

This PR fixes useDocker parsing from given boolean string (e.g. true/false string value) to a boolean value in JavaScript/TypeScript.

I noticed a problem with the parsing when trying to use this GitHub Action (v1.0.7) to deploy an app to Heroku using Git. My app does not use Dockerfile, but my deployment workflow that using this GitHub Action always attempted to deploy the app as Docker container even though I have set useDocker to false in my workflow. After I looked up the source code of this GitHub Action, I think the problem was at how useDocker is parsed.

Originally, useDocker was assigned by reading the string value from the input and parsed by instantiating a new Boolean object with string value as input for the Boolean's constructor. After that, the Boolean object is evaluated in the conditional if. The result of the evaluation always return true boolean value, hence the deployment always choose to Docker instead of Git.

I implemented a fix by replacing core.getInput() with core.getBooleanInput(). According to @actions/core documentation, core.getBooleanInput() will parse the string in the input to its correct boolean value representation. I think this approach is more intuitive than the original version where the input need to be parsed into a Boolean object constructor.

ElayGelbart commented 2 years ago

hey @addianto, first of all thank you for your contribution. i will merge this PR and release new version :)

addianto commented 2 years ago

Awesome. Thanks @ElayGelbart. 🙏