fix: standardize bond link selection method JSON field naming - #2491
Conversation
Adds support for "linkSelectMethod" as the JSON configuration field name while maintaining backward compatibility with the legacy "activeReselect" field name. This change aligns the JSON field name with the internal API naming convention (setLinkSelectMethod/getLinkSelectMethod) and follows the established pattern in the codebase where JSON field names match their corresponding setter method names. The implementation: - Checks for the new field name first, then falls back to the legacy name - Emits a deprecation warning when "activeReselect" is used - Ensures existing configurations continue to work without modification Resolves terminology inconsistency identified in docs PR #263 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <noreply@anthropic.com>
There was a problem hiding this comment.
Pull Request Overview
This PR standardizes the JSON field naming for bond link selection method configuration to improve API consistency. The change introduces "linkSelectMethod" as the preferred field name while maintaining backward compatibility with the existing "activeReselect" field.
Key changes:
- Adds support for "linkSelectMethod" field name with fallback to legacy "activeReselect"
- Implements deprecation warning for legacy field usage
- Maintains full backward compatibility for existing configurations
| } else { | ||
| linkSelectMethodStr = OSUtils::jsonString(customPolicy["activeReselect"], "always"); | ||
| if (customPolicy.contains("activeReselect")) { | ||
| fprintf(stderr, "warning: 'activeReselect' is deprecated, please use 'linkSelectMethod' instead in policy '%s'\n", customPolicyStr.c_str()); |
There was a problem hiding this comment.
The deprecation warning should use a consistent logging mechanism instead of fprintf(stderr). Consider using the same logging system used elsewhere in the codebase for consistency.
| fprintf(stderr, "warning: 'activeReselect' is deprecated, please use 'linkSelectMethod' instead in policy '%s'\n", customPolicyStr.c_str()); | |
| OSUtils::log("warning", "'activeReselect' is deprecated, please use 'linkSelectMethod' instead in policy '%s'", customPolicyStr.c_str()); |
| linkSelectMethodStr = OSUtils::jsonString(customPolicy["linkSelectMethod"], "always"); | ||
| } else { | ||
| linkSelectMethodStr = OSUtils::jsonString(customPolicy["activeReselect"], "always"); | ||
| if (customPolicy.contains("activeReselect")) { |
There was a problem hiding this comment.
The check for 'activeReselect' is redundant since OSUtils::jsonString already handles missing keys by returning the default value. The warning will be printed even when the field doesn't exist.
| if (customPolicy.contains("activeReselect")) { | |
| if (linkSelectMethodStr != "always") { |
joseph-henry
left a comment
joseph-henry
left a comment
There was a problem hiding this comment.
This looks great, thanks for syncing this up and fixing it.
Adds support for "linkSelectMethod" as the JSON configuration field name while maintaining backward compatibility with the legacy "activeReselect" field name.
This change aligns the JSON field name with the internal API naming convention (setLinkSelectMethod/getLinkSelectMethod) and follows the established pattern in the codebase where JSON field names match their corresponding setter method names.
The implementation:
Resolves terminology inconsistency identified in docs PR #263
🤖 Generated with Claude Code