fix: add x-go-name for server urls#2304
Conversation
Greptile SummaryThis PR adds
Confidence Score: 4/5Safe to merge after addressing the silent error-swallowing in server_urls.go One P1 finding: the extParseGoFieldName error is silently dropped, which is inconsistent with all other x-go-name handling in the codebase and can produce silently wrong output for malformed specs. The rest of the change is correct and well-tested. pkg/codegen/server_urls.go — error from extParseGoFieldName should be propagated, not swallowed
|
| Filename | Overview |
|---|---|
| pkg/codegen/server_urls.go | Adds x-go-name support for server URL naming; error from extParseGoFieldName is silently swallowed (inconsistent with all other x-go-name call sites that propagate the error) |
| examples/generate/serverurls/api.yaml | Adds a new server entry demonstrating x-go-name usage; looks correct |
| examples/generate/serverurls/gen_test.go | Adds a simple test asserting the x-go-name-derived constant has the correct URL value |
Reviews (1): Last reviewed commit: "fix: add x-go-name for server urls" | Re-trigger Greptile
do not swallow the extGoName error from automated review bot Co-authored-by: greptile-apps[bot] <165735046+greptile-apps[bot]@users.noreply.github.com>
|
Thanks. |
|
Fyi, your bot responded that the original implementation was actually correct. I looked at the code and think the original suggestion to not swallow the error seems to be used and the follow-up is incorrect. |
|
I can create a follow-up to this if you need me to |
|
Nah, it's ok. It's not a place where it's important. |
This allows one to use the
x-go-namein the servers list. I wanted to customize the name instead of generating it from the description. I was able to do this with aserver-urls.tmplas a workaround. This seems like a simple enough feature that it could be implemented as the default behavior, which is why I also did not create an issue for it first. Although I could do that if you feel like it would be useful.