Skip to content
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

update go conf.json to output valid go module #2

Closed
wants to merge 2 commits into from

Conversation

dominicbarnes
Copy link

I am looking to use a robust Go client for PokeAPI, when I came across this OpenAPI specification of yours.

Upon trying to import the generated Go client, I run into the following error:

go: finding module for package github.com/cliffano/pokeapi-clients/clients/go/generated
go: found github.com/cliffano/pokeapi-clients/clients/go/generated in github.com/cliffano/pokeapi-clients/clients/go/generated v0.0.0-20230225003110-e07a08773682
go: pokemon-checklists/cmd/import imports
	github.com/cliffano/pokeapi-clients/clients/go/generated: github.com/cliffano/pokeapi-clients/clients/go/generated@v0.0.0-20230225003110-e07a08773682: parsing go.mod:
	module declares its path as: github.com/GIT_USER_ID/GIT_REPO_ID
	        but was required as: github.com/cliffano/pokeapi-clients/clients/go/generated

This is easily fixed by removing the GIT_USER_ID and GIT_REPO_ID placeholders, which I accomplished by updating the relevant conf.json.

This same change is likely also needed for the other go-* generated modules, but I wanted to keep my PR initially scoped to only what blocks me, though I can easily apply the change more broadly.

I'm also unsure if I should run make generate and commit those changes too, as it seemed entirely possible that it is automated.

@dominicbarnes
Copy link
Author

I did attempt to re-generate the go client using the following command:

make generate LANGS_ALL=go APP_BASE_DIR=`pwd` LOCAL_SPEC_PATH=specification/pokeapi.yml

The resulting diff was much larger than I expected (+68745/-2128), so I was hesitant to commit the changes at first.

After looking more closely though, this seems to be because the specification was updated significantly after the last time clients were generated. As such, I've gone ahead and committed the changes.

@cliffano
Copy link
Member

Thanks for raising this issue @dominicbarnes

There are a few things here:

  • The missing git user and repo details.
    Thanks for bringing this to my attention, I have added git user and repo details to Swaggy-C cliffano/swaggy-c@88492c1 , which is the common Makefile that I use across my OpenAPI-based API client repos.
    So I'm going to merge your PR first, and then I'll upgrade to Swaggy C 3.x and regenerate the clients.

  • The significant spec update.
    Due to the gap in the initial specification, I utilised ChatGPT to generate the missing parts of PokeAPI OpenAPI specification on the day ChatGPT was made available publicly. And later on I learnt that LLMs could easily hallucinate.
    I haven't had the chance to analyse how drunk ChatGPT was, but I'll likely revert the large updates in the specification file itself.
    I reckon the proper way to complete the spec is really to parse the hand-built Python SDK.

cliffano added a commit to cliffano/swaggy-c that referenced this pull request Apr 25, 2024
@cliffano
Copy link
Member

I forgot that this issue is still open.

Added credit to @dominicbarnes at https://github.com/cliffano/swaggy-c/blob/main/CHANGELOG.md#added .
Many thanks!

Closing this issue now that the Git SCM details are added to Swaggy C.

@cliffano cliffano closed this Apr 25, 2024
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.

2 participants