Skip to content

Conversation

@emann
Copy link
Collaborator

@emann emann commented Jan 29, 2021

No description provided.

@emann emann requested a review from dbanty January 29, 2021 21:06
Copy link
Contributor

@dbanty dbanty left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the biggest suggestion is for us to use and recommend (in the example) versioning. IDK if GHA supports semantic versions or if we're just only going to update that for breaking changes.

README.md Outdated
# Use the action to generate a client package
# This uses all defaults (latest version, openapi.json in the current workspace, no configuration)
- name: Generate Python Client
uses: triaxtec/openapi-python-client-action@main
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If we're going to publish this we should version it and do @v1 instead I think. I've seen several times that that is best practice so if we make breaking changes we don't break anyone's CI.

@@ -0,0 +1,27 @@
name: "Generate A Python Client Package From An OpenAPI file"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this the name that gets displayed in the marketplace? Or just in Actions when running?

If the first, this should be "openapi-python-client Action" or something. If the second, maybe just "Generate Python Client"?

Copy link
Collaborator Author

@emann emann Feb 1, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The former; My thinking was to make it a tad more search friendly (on the actions marketplace that is) so that its a bit discoverable + easy to get what it does just from reading the name. That seems to be the standard way of doing it based off of what I've seen in the marketplace - unless its a multipurpose/function action the name briefly tells you what it does. I think it could do with being shortened however - how do you feel about just "Generate Python Client From An OpenAPI File"?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Or "Generate Python Client from OpenAPI"?

entrypoint.sh Outdated
config_arg=""
fi

pipx run ${version_arg} openapi-python-client ${config_arg} generate --path ${openapi_file_path} No newline at end of file
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Might be better to pipx install this in the Dockerfile, then reuse the same version? Seems unlikely that someone would generate multiple clients in a single CI pipeline but we've done weirder things.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

AFAIK theres no way to use inputs inside the Dockerfile unfortunately :(

emann and others added 2 commits February 1, 2021 17:22
Co-authored-by: Dylan Anthony <43723790+dbanty@users.noreply.github.com>
@emann emann requested a review from dbanty February 1, 2021 22:48
@emann emann merged commit 6630ed2 into main Feb 3, 2021
@dbanty dbanty deleted the develop branch October 7, 2021 02:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants