Simplify GitHub Action entrypoint (#1909) #2119
Conversation
|
@cooperlees Please commit the above suggestion to make this change. Thank you! |
This deprecation should be visible in GitHub Action's UI now. Co-authored-by: Shota Ray Imaki <[email protected]>
|
If no one objects before then, I'll be merging this in 30ish minutes. Cooperlees already approved and his non-binding comment was also resolved. Primer is just failing because this doesn't have the change noting how tox is actually expecting no changes. |
|
Thank you @simaki for your contribution! This should help keep this action maintainable and easy to understand which is always nice since not everyone on the core team is great at this kind of stuff (e.g. me!) |
There was a problem hiding this comment.
@ichard26 Hi, could you kindly merge these fixes? I noticed these mistakes by my local run. Sorry about bothering you... Many thanks! (btw continuous testing of GH Actions would be nice but I've got no idea how to do it. seeking idea.)
| if [ -n $INPUT_BLACK_ARGS ]; then | ||
| echo '::warning::Input `with.black_args` is deprecated. Use `with.options` and `with.src` instead.' | ||
| black $INPUT_BLACK_ARGS | ||
| exit $? |
There was a problem hiding this comment.
fi is missing here...
| exit $? | |
| exit $? | |
| fi |
| # Default (if no args provided). | ||
| black_args+=("--check" "--diff") | ||
| fi | ||
| if [ -n $INPUT_BLACK_ARGS ]; then |
There was a problem hiding this comment.
"" is necessary to check if $INPUT_BLACK_ARGS is empty.
| if [ -n $INPUT_BLACK_ARGS ]; then | |
| if [ -n "$INPUT_BLACK_ARGS" ]; then |
yeah don't worry about, I'm honestly more surprised I didn't catch that error 😞, here's the hotfix: #2202 I got some ideas to test out our Action but they will have to wait till later once I get some time. |
ghost
deleted the
This comment has been minimized.
This comment has been minimized.
This PR simplifies
entrypoint.shfor GitHub Actions by removing duplication ofargsandblack_args(cf. #1909).The reason why #1909 uses the input id
black_argsis to avoid an overlap withargs, but this naming seems redundant.So let me suggest
optionandsrc, which are consistent with CLI. Backward compatibility is guaranteed; Users can still useblack_argsas well.Any suggestions for improvements are more than welcome. Thanks!
This PR may not require a changelog entry.