Copilot commented on code in PR #1469:
URL: https://github.com/apache/pulsar-client-go/pull/1469#discussion_r2905318583
##########
pulsaradmin/pkg/admin/topic.go:
##########
@@ -1027,12 +1028,36 @@ type Topics interface {
// RemoveAutoSubscriptionCreationWithContext Remove auto subscription
creation override for a topic
RemoveAutoSubscriptionCreationWithContext(context.Context,
utils.TopicName) error
- // GetSchemaCompatibilityStrategy returns schema compatibility strategy
for a topic
+ // GetSchemaCompatibilityStrategy returns the applied schema
compatibility strategy for a topic.
GetSchemaCompatibilityStrategy(utils.TopicName)
(utils.SchemaCompatibilityStrategy, error)
- // GetSchemaCompatibilityStrategyWithContext returns schema
compatibility strategy for a topic
+ // GetSchemaCompatibilityStrategyWithContext returns the applied schema
compatibility strategy for a topic.
GetSchemaCompatibilityStrategyWithContext(context.Context,
utils.TopicName) (utils.SchemaCompatibilityStrategy, error)
+ // GetSchemaCompatibilityStrategyApplied returns schema compatibility
strategy for a topic.
+ //
+ // @param topic
+ // topicName struct
+ // @param applied
+ // when set to true, function will try to find policy applied to
this topic
+ // in namespace or broker level, if no policy set in topic level
+ GetSchemaCompatibilityStrategyApplied(utils.TopicName, bool)
(utils.SchemaCompatibilityStrategy, error)
Review Comment:
The method name `GetSchemaCompatibilityStrategyApplied` is misleading
because it accepts `applied=false` (which returns the *non-applied* /
topic-level value). Consider renaming to something that reflects the boolean
option (e.g., `GetSchemaCompatibilityStrategyWithApplied` /
`GetSchemaCompatibilityStrategyWithOptions`) to avoid confusing API consumers.
##########
pulsaradmin/pkg/admin/topic.go:
##########
@@ -1027,12 +1028,36 @@ type Topics interface {
// RemoveAutoSubscriptionCreationWithContext Remove auto subscription
creation override for a topic
RemoveAutoSubscriptionCreationWithContext(context.Context,
utils.TopicName) error
- // GetSchemaCompatibilityStrategy returns schema compatibility strategy
for a topic
+ // GetSchemaCompatibilityStrategy returns the applied schema
compatibility strategy for a topic.
GetSchemaCompatibilityStrategy(utils.TopicName)
(utils.SchemaCompatibilityStrategy, error)
- // GetSchemaCompatibilityStrategyWithContext returns schema
compatibility strategy for a topic
+ // GetSchemaCompatibilityStrategyWithContext returns the applied schema
compatibility strategy for a topic.
GetSchemaCompatibilityStrategyWithContext(context.Context,
utils.TopicName) (utils.SchemaCompatibilityStrategy, error)
+ // GetSchemaCompatibilityStrategyApplied returns schema compatibility
strategy for a topic.
+ //
+ // @param topic
+ // topicName struct
+ // @param applied
+ // when set to true, function will try to find policy applied to
this topic
+ // in namespace or broker level, if no policy set in topic level
+ GetSchemaCompatibilityStrategyApplied(utils.TopicName, bool)
(utils.SchemaCompatibilityStrategy, error)
+
+ // GetSchemaCompatibilityStrategyAppliedWithContext returns schema
compatibility strategy for a topic.
+ //
+ // @param ctx
+ // context used for the request
+ // @param topic
+ // topicName struct
+ // @param applied
+ // when set to true, function will try to find policy applied to
this topic
+ // in namespace or broker level, if no policy set in topic level
+ GetSchemaCompatibilityStrategyAppliedWithContext(
+ context.Context,
+ utils.TopicName,
+ bool,
+ ) (utils.SchemaCompatibilityStrategy, error)
Review Comment:
Same naming concern for `GetSchemaCompatibilityStrategyAppliedWithContext`:
the `Applied` suffix reads like it always returns the applied value, but
`applied=false` changes behavior. Renaming to reflect the option would make the
API clearer and consistent with other topic getters that take an `applied bool`
flag.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
