-
Notifications
You must be signed in to change notification settings - Fork 5
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
Generate Inputs and Outputs in the README #40
Conversation
ba998ed
to
323edcf
Compare
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.
So cool!
@@ -0,0 +1,11 @@ | |||
{ | |||
"dependencies": { | |||
"action-docs": "git+https://github.com/deemp/action-docs.git#0cb0ac2c40736aba24c4fd5097f1074b5a7c7a4b" |
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.
🤯
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.
Well, if npm update action-docs
works, the SHA can be removed.
This may speed up builds in subsequent runs at the expense of slightly-longer | ||
builds when a full cache-hit occurs. (Since `@v4.2.0`) | ||
|
||
| name | description | required | default | |
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.
One thing I noticed when I was working on this myself is that a value with a default
is not "required", even though we put required: true
in the action.yml
. Ideally, that would be reflected here by setting the table cell to something like required && default != null
.
Have you thought about that at all?
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.
Have you thought about that at all?
No, I haven't. Could you please provide an example of such a bad conversion?
required && default != null.
So, the truth table will look like this? Why should it look so?
required | default != null | final required |
---|---|---|
true | true | true |
true | false | false |
false | true | false |
false | false | false |
I think the current version where values don't depend on each other is more straightforward. I can document that default
defaults to ''
and required
defaults to 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.
Could you please provide an example of such a bad conversion?
I wouldn't call it a "bad conversion", but every single input in the table you've generated here says true
in the required column -- even though none of them are actually required (because they have defaults).
I think the current version where values don't depend on each other is more straightforward
Yeah I agree. If you have nothing clever, I will probably just start removing the required
key from any inputs that have defaults. There was a reason I started doing it that way, but now I can't remember.
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 you have nothing clever,
Yes, I don't have anything.
Supersedes #38.
Run:
Generated README - link
N.B. The reference in
package.json
should be updated after npalm/action-docs#505 is merged.