-
-
Notifications
You must be signed in to change notification settings - Fork 116
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
chore: change access_token
to optional
#72
Conversation
@@ -15,10 +15,10 @@ inputs: | |||
access_token: | |||
description: 'Your GitHub Access Token, defaults to: {{ github.token }}' | |||
default: '${{ github.token }}' | |||
required: true | |||
required: false |
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.
It has a default value
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.
What about the required
prop in index.ts
?
cancel-workflow-action/src/index.ts
Line 33 in cd25b7a
const token = core.getInput('access_token', { required: 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.
@styfle Should it be deleted?
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.
I'm not sure. Are either of these required props causing problems?
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.
They're not causing any problems to me, but I saw it's set to required although it has a default value so that's why I created this PR
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.
If its not causing problems, lets not change it 🙂
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.
I would argue that it also implies that the docs can be simplified, if this works without passing the default.
@styfle Is there anything I can do to get this one merged? |
@styfle This is valuable PR since it simplifies action usage. Please consider merging it. |
action.yml
access_token
to optional
No description provided.