Skip to content

Conversation

@adamdickmeiss
Copy link
Contributor

@adamdickmeiss adamdickmeiss commented Jan 23, 2026

https://index-data.atlassian.net/browse/CROSSLINK-201

The directory.Entry definition is used in most places including as type for custom_data in sqlc mapping.

This means fewer map inspections in a number of places.

Copilot AI review requested due to automatic review settings January 23, 2026 12:47
@adamdickmeiss adamdickmeiss marked this pull request as draft January 23, 2026 12:47
@adamdickmeiss adamdickmeiss changed the title Crosslink 201 lms config directory CROSSLINK-201 LMS config directory Jan 23, 2026
Copy link
Contributor

Copilot AI left a 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.LmsConfig type instead of map[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

@adamdickmeiss adamdickmeiss marked this pull request as ready for review January 26, 2026 13:58
@adamdickmeiss adamdickmeiss marked this pull request as draft January 26, 2026 14:27
Copy link
Contributor

Copilot AI left a 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.

adamdickmeiss and others added 6 commits January 26, 2026 18:15
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>
@adamdickmeiss adamdickmeiss marked this pull request as ready for review January 26, 2026 17:54
@adamdickmeiss adamdickmeiss merged commit e6f5566 into main Jan 27, 2026
4 checks passed
@adamdickmeiss adamdickmeiss deleted the CROSSLINK-201-lms-config-directory branch January 27, 2026 14:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

3 participants