octokit / source-generator

6 stars 3 forks source link

Hack: fix package name in api_client.go #20

Closed kfcampbell closed 7 months ago

kfcampbell commented 8 months ago

We have a post-processing step that changes the package name in the generated top-level api_client.go file:

https://github.com/octokit/source-generator/blob/d1c29eea0c06262a42cc119b3ab0304f5e8e67ef/post-processors/go/main.go#L254-L264

In Go, files in the same directory must be in the same package. When this processing step is removed, the following error is produced: found packages kiota (api_client.go) and main (main.go) in /home/kfcampbell/github/dev/source-generator/generated/go. This would be fine if we were implementing a library only, without a main.go file.

One option would be to settle on a permanent repository to push our generated code to, and we can then represent our sample app in whatever repo we like. Another option would be to attempt to use Kiota's --class-name flag to set the package name in just this file to main (if that's possible). A third option would be to take this issue to the Kiota team and see if they have suggestions. Last, we could resign ourselves to the need for a post-processor here (it's not adding much time or complexity to the generative procedure) and close this issue.

baywet commented 8 months ago

This is driven by another option -n ( --namespace-name) where you can pass github.com/octokit/main if you want.

kfcampbell commented 8 months ago

Likely what we'll do here is lift our sample app out of the root directory of the generated code and host it in another repository. At some point in the near future we'll host the generated code in its own repo and have a bot update it periodically based on OpenAPI updates and Kiota dependency updates.

When creating Go code that's intended only as a library (as opposed to providing entry point(s) for applications) the convention is to eschew package main as Kiota correctly does here. The hack to move the api_client.go package is only relevant for the specific use case we've temporarily constructed here.

This is blocked pending #26.