-
Notifications
You must be signed in to change notification settings - Fork 0
CROSSLINK-201 LMS config directory #375
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
Conversation
customData still the same type. directory.Entries is generated from the directory OpenAPI spec.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This PR refactors the LMS (Library Management System) configuration in the Directory service by renaming the "ncip" field to "lmsConfig" and restructuring the configuration schema. The changes move from a map-based configuration approach to a strongly-typed OpenAPI schema with generated Go types.
Changes:
- Renamed and restructured the LMS configuration from "ncip" to "lmsConfig" in the OpenAPI schema with camelCase naming
- Refactored LMS adapter to use generated
directory.LmsConfigtype instead ofmap[string]any - Updated all references and tests to use the new configuration structure
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 8 comments.
Show a summary per file
| File | Description |
|---|---|
| directory/directory_api.yaml | Renamed "Ncip" schema to "LmsConfig", changed field names from snake_case to camelCase, and updated field types and defaults |
| broker/lms/lms_adapter_ncip.go | Refactored to use directory.LmsConfig type, removed manual parsing logic, simplified configuration handling |
| broker/lms/lms_creator_impl.go | Added JSON marshaling/unmarshaling to convert CustomData to directory.Entry type |
| broker/lms/lms_adapter_test.go | Removed configuration parsing tests, updated existing tests to use new directory.LmsConfig structure |
| broker/lms/lms_creator_test.go | Updated test data to use "LmsConfig" instead of "ncip" field name |
| broker/ncipclient/ncipclient.go | Removed unused constant definitions that were moved/refactored |
| broker/adapter/api_directory.go | Updated to use generated directory.EntriesResponse and directory.Entry types with proper field access |
…ry' into CROSSLINK-201-lms-config-directory
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
Copilot reviewed 18 out of 18 changed files in this pull request and generated 5 comments.
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
https://index-data.atlassian.net/browse/CROSSLINK-201
The
directory.Entrydefinition is used in most places including as type forcustom_datain sqlc mapping.This means fewer map inspections in a number of places.