feat(internal/librarian/golang): restructure Go configuration in librarian.yaml#5975
Conversation
410a7d7 to
038b25f
Compare
There was a problem hiding this comment.
Code Review
This pull request refactors the configuration schema by nesting Go-specific API settings directly within the API struct, replacing the previous GoAPIs list in the GoModule. The changes include updates to the configuration model, documentation, and core processing logic, along with a comprehensive update of the test suite to align with the new structure. Review feedback recommends improving the Tidy function to clear the Path field in the nested Go configuration when it matches the parent API path to minimize redundancy in serialized YAML.
There was a problem hiding this comment.
Code Review
This pull request refactors the configuration schema by moving Go-specific API settings from a global list in GoModule to a nested field within each API entry. This change simplifies the configuration structure and is reflected across the codebase, including documentation, configuration structs, generation logic, and extensive test updates. A review comment identifies an inefficiency and a potential bug in internal/librarian/golang/module.go where findGoAPI is used instead of direct access to the now-nested api.Go field.
zhumin8
left a comment
There was a problem hiding this comment.
nit: maybe use refactor or fix as. title type
This removes support for top-level Go defaults (Issue googleapis#5246) to focus this branch solely on nesting Go configuration under API entries (Issue googleapis#5471).
With Go configuration now nested directly under API entries (Issue googleapis#5471), the API path is always known from the containing API structure. Removing GoAPI.Path eliminates redundancy and prevents potential state drift.
4f72492 to
b4ab8ae
Compare
…arian.yaml (#6015) This PR re-applies the Go configuration restructuring from PR #5975, nesting Go configuration under API entries. This PR will be merged right before the companion configuration migration in google-cloud-go: googleapis/google-cloud-go#14587 is merged. <details> <summary>This temporarily skips the integration test using google-cloud-go.</summary> Here is how GitHub Actions handles the subsequent `create-issue-on-failure` job: ```yaml create-issue-on-failure: needs: [integration] if: ${{ always() && contains(needs.*.result, 'failure') && github.event_name == 'push' && github.ref == 'refs/heads/main' }} ``` 1. **Dependency State**: Because `integration` has `if: false`, GitHub Actions marks its final status as `skipped`. 2. **Evaluation**: By default, a job is skipped if its `needs` dependency is skipped. However, because `create-issue-on-failure` uses `always()`, GitHub evaluates its `if:` condition. 3. **Result**: The expression `needs.*.result` evaluates to `['skipped']`. Since this array does not contain `'failure'`, the `contains(...)` check evaluates to `false`. As a result, the `create-issue-on-failure` job will be cleanly skipped without creating any false alarm issues or causing workflow errors. </details> Fixes #5471
(I forgot to submit comment.) I went ahead with feat because this requires changes in Go's configuration file. |
This PR restructures Go configuration in
librarian.yamlto support nestingAPI.Gounderapisentries.Fixes #5471