alstr / todo-to-issue-action

Action that converts TODO comments to GitHub issues on push.
MIT License
630 stars 121 forks source link

Bug: Workflow validation fails because optional inputs are set to required. #125

Closed sammcj closed 2 years ago

sammcj commented 2 years ago

Because a number of the inputs are set to required: true, workflow validation fails if they're not provided.

  BEFORE:
    description: 'The SHA of the last pushed commit (automatically set)'
    required: true
    default: '${{ github.event.before || github.base_ref }}'

You can see here that all required inputs must be provided, and one that I haven't provided fails validation:

image

What I would suggest is changing those inputs to required: false, and make the code handle the defaults.

alstr commented 2 years ago

Hi there. That's strange behaviour for sure. The workflow file for this repo doesn't provide any of them, and it runs fine:

https://github.com/alstr/todo-to-issue-action/blob/master/.github/workflows/workflow.yml

You shouldn't provide any inputs except those mentioned in the setup guide: https://github.com/alstr/todo-to-issue-action#setup Providing SHA, BEFORE etc. will result in certain confusion. All of those are supplied automatically. Please could you try removing everything and then only add those that should be? Thanks!

sammcj commented 2 years ago

It will work - but behind the scenes Github will be showing warnings (they may show up with debug runs enabled), but peoples IDEs/editors will warn about missing inputs (unless they have broken YAML validation for Github-Workflow schemas).

sammcj commented 2 years ago

The docs are a bit confusing but Github basically suggest that defaults are for optional inputs.

https://github.com/actions/toolkit/tree/main/packages/core#inputsoutputs

alstr commented 2 years ago

My apologies, I misunderstood your original post. I see what you mean now by validation rather than the actual workflow failing.

This seems like a good fix. I agree the docs are confusing. The inputs are required, just not from the user. The code should handle default values to some extent, but need to verify that. If you do want to submit a PR, that’d be great.