[fix][broker] Delete unused param #20866
Conversation
| @@ -2217,7 +2217,6 @@ public void setReplicationClusters( | |||
| public void removeReplicationClusters(@Suspended final AsyncResponse asyncResponse, | |||
| @PathParam("tenant") String tenant, @PathParam("namespace") String namespace, | |||
| @PathParam("topic") @Encoded String encodedTopic, | |||
| @QueryParam("backlogQuotaType") BacklogQuotaType backlogQuotaType, | |||
There was a problem hiding this comment.
Is this change backward compatible, like clients that used to send request and succeed won't suddenly fail? If not, this change needs doc-required
There was a problem hiding this comment.
I second @JooHyukKim's suggestion. For compatiblity we should not remove param.
There was a problem hiding this comment.
@JooHyukKim @tisonkun It is a QueryParam, and it is optional, I don't think there are compatibility issues. If we delete this QueryParam and user send a request with 'backlogQuotaType', it works well.
There was a problem hiding this comment.
Oh, that's great 👍🏻
Also, it will be nice if can keep consistencey with REST API documentation, maybe check doc-required instead?
There was a problem hiding this comment.
Make sense. Thanks.
This comment was marked as outdated.
This comment was marked as outdated.
github-actions
bot
added
doc-required
|
The pr had no activity for 30 days, mark with Stale label. |
Codecov Report
@@ Coverage Diff @@
## master #20866 +/- ##
=============================================
+ Coverage 36.75% 73.20% +36.45%
- Complexity 12193 32475 +20282
=============================================
Files 1698 1887 +189
Lines 130430 140117 +9687
Branches 14250 15426 +1176
=============================================
+ Hits 47940 102574 +54634
+ Misses 76167 29442 -46725
- Partials 6323 8101 +1778
Flags with carried forward coverage won't be shown. Click here to find out more.
|
|
@Technoboy- @nodece may you do another check? |
nodece
added
the
type/cleanup
Modifications
Delete unused param in removeReplicationClusters.
Verifying this change
(Please pick either of the following options)
This change is a trivial rework / code cleanup without any test coverage.
(or)
This change is already covered by existing tests, such as (please describe tests).
(or)
This change added tests and can be verified as follows:
(example:)
Does this pull request potentially affect one of the following parts:
If the box was checked, please highlight the changes
Documentation
docdoc-requireddoc-not-neededdoc-completeMatching PR in forked repository
PR in forked repository: