Re: [PR] fix: toCommaDelimitedString returns "null,null" when inputs are null [dubbo]
AlbumenJ merged PR #15338: URL: https://github.com/apache/dubbo/pull/15338 -- 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: notifications-unsubscr...@dubbo.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: notifications-unsubscr...@dubbo.apache.org For additional commands, e-mail: notifications-h...@dubbo.apache.org
Re: [PR] fix: toCommaDelimitedString returns "null,null" when inputs are null [dubbo]
ankitshokeen commented on code in PR #15338:
URL: https://github.com/apache/dubbo/pull/15338#discussion_r2078169754
##
dubbo-common/src/main/java/org/apache/dubbo/common/utils/StringUtils.java:
##
@@ -1242,14 +1242,17 @@ public static byte decodeHexByte(CharSequence s, int
pos) {
}
/**
- * Create the common-delimited {@link String} by one or more {@link
String} members
+ * Creates a comma-delimited string from one or more string values.
*
- * @param oneone {@link String}
- * @param others others {@link String}
- * @return null if one or others is
null
+ * @param onethe first string value
+ * @param others additional string values
+ * @return a combined comma-delimited string, or null if
{@code one} or {@code others} is {@code null}
* @since 2.7.8
*/
public static String toCommaDelimitedString(String one, String... others) {
+if (one == null || others == null) {
+return null;
Review Comment:
yes, it should return one
--
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: notifications-unsubscr...@dubbo.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
-
To unsubscribe, e-mail: notifications-unsubscr...@dubbo.apache.org
For additional commands, e-mail: notifications-h...@dubbo.apache.org
Re: [PR] fix: toCommaDelimitedString returns "null,null" when inputs are null [dubbo]
AlbumenJ commented on code in PR #15338:
URL: https://github.com/apache/dubbo/pull/15338#discussion_r2076650834
##
dubbo-common/src/main/java/org/apache/dubbo/common/utils/StringUtils.java:
##
@@ -1242,14 +1242,17 @@ public static byte decodeHexByte(CharSequence s, int
pos) {
}
/**
- * Create the common-delimited {@link String} by one or more {@link
String} members
+ * Creates a comma-delimited string from one or more string values.
*
- * @param oneone {@link String}
- * @param others others {@link String}
- * @return null if one or others is
null
+ * @param onethe first string value
+ * @param others additional string values
+ * @return a combined comma-delimited string, or null if
{@code one} or {@code others} is {@code null}
* @since 2.7.8
*/
public static String toCommaDelimitedString(String one, String... others) {
+if (one == null || others == null) {
+return null;
Review Comment:
if one != null && others == null
should return one?
--
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: notifications-unsubscr...@dubbo.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
-
To unsubscribe, e-mail: notifications-unsubscr...@dubbo.apache.org
For additional commands, e-mail: notifications-h...@dubbo.apache.org
Re: [PR] fix: toCommaDelimitedString returns "null,null" when inputs are null [dubbo]
codecov-commenter commented on PR #15338: URL: https://github.com/apache/dubbo/pull/15338#issuecomment-2832024536 ## [Codecov](https://app.codecov.io/gh/apache/dubbo/pull/15338?dropdown=coverage&src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) Report Attention: Patch coverage is `0%` with `2 lines` in your changes missing coverage. Please review. > Project coverage is 29.35%. Comparing base [(`c393896`)](https://app.codecov.io/gh/apache/dubbo/commit/c393896ca822271f13147edd3603a83053833704?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) to head [(`55ddfa4`)](https://app.codecov.io/gh/apache/dubbo/commit/55ddfa4570c6b81060ffe73f69cbead289392a6f?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache). | [Files with missing lines](https://app.codecov.io/gh/apache/dubbo/pull/15338?dropdown=coverage&src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Patch % | Lines | |---|---|---| | [...ava/org/apache/dubbo/common/utils/StringUtils.java](https://app.codecov.io/gh/apache/dubbo/pull/15338?src=pr&el=tree&filepath=dubbo-common%2Fsrc%2Fmain%2Fjava%2Forg%2Fapache%2Fdubbo%2Fcommon%2Futils%2FStringUtils.java&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-ZHViYm8tY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9kdWJiby9jb21tb24vdXRpbHMvU3RyaW5nVXRpbHMuamF2YQ==) | 0.00% | [2 Missing :warning: ](https://app.codecov.io/gh/apache/dubbo/pull/15338?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | > :exclamation: There is a different number of reports uploaded between BASE (c393896) and HEAD (55ddfa4). Click for more details. > > HEAD has 2 uploads less than BASE > >| Flag | BASE (c393896) | HEAD (55ddfa4) | >|--|--|--| >|integration-tests|1|0| >|unit-tests|1|0| > Additional details and impacted files ```diff @@ Coverage Diff @@ ##3.3 #15338 +/- ## = - Coverage 60.76% 29.35% -31.42% + Complexity10916 8939 -1977 = Files 1886 1886 Lines 8612786129+2 Branches 1290312903 = - Hits 5233525279-27056 - Misses2833556864+28529 + Partials 5457 3986 -1471 ``` | [Flag](https://app.codecov.io/gh/apache/dubbo/pull/15338/flags?src=pr&el=flags&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Coverage Δ | | |---|---|---| | [integration-tests](https://app.codecov.io/gh/apache/dubbo/pull/15338/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `?` | | | [samples-tests](https://app.codecov.io/gh/apache/dubbo/pull/15338/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `29.35% <0.00%> (-0.02%)` | :arrow_down: | | [unit-tests](https://app.codecov.io/gh/apache/dubbo/pull/15338/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `?` | | Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#carryforward-flags-in-the-pull-request-comment) to find out more. [:umbrella: View full report in Codecov by Sentry](https://app.codecov.io/gh/apache/dubbo/pull/15338?dropdown=coverage&src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache). :loudspeaker: Have feedback on the report? [Share it here](https://about.codecov.io/codecov-pr-comment-feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache). :rocket: New features to boost your workflow: - :snowflake: [Test Analytics](https://docs.codecov.com/docs/test-analytics): Detect flaky tests, report on failures, and find test suite problems. - :package: [JS Bundle Analysis](https://docs.codecov.com/docs/javascript-bundle-analysis): Save yourself from yourself by tracking and limiting bundle sizes in JS merges. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to Gi
[PR] fix: toCommaDelimitedString returns "null,null" when inputs are null [dubbo]
ankitshokeen opened a new pull request, #15338: URL: https://github.com/apache/dubbo/pull/15338 ## Issue https://github.com/apache/dubbo/issues/15321 ## What is the purpose of the change? This change corrects the behavior of StringUtils.toCommaDelimitedString(String one, String... others) to return null when either argument is null, as described in its Javadoc. Previously, the method would return a string like "null,null", which could lead to misleading outputs and test failures. This update ensures consistency between the method's implementation and its documented contract. ## Checklist - [x] Make sure there is a [GitHub_issue](https://github.com/apache/dubbo/issues) field for the change. - [x] Write a pull request description that is detailed enough to understand what the pull request does, how, and why. - [x] Write necessary unit-test to verify your logic correction. If the new feature or significant change is committed, please remember to add sample in [dubbo samples](https://github.com/apache/dubbo-samples) project. - [x] Make sure gitHub actions can pass. [Why the workflow is failing and how to fix it?](../CONTRIBUTING.md) -- 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: notifications-unsubscr...@dubbo.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: notifications-unsubscr...@dubbo.apache.org For additional commands, e-mail: notifications-h...@dubbo.apache.org
Re: [PR] fix: toCommaDelimitedString returns "null,null" when inputs are null [dubbo]
ankitshokeen commented on PR #15322: URL: https://github.com/apache/dubbo/pull/15322#issuecomment-2832017015 > Why are there so many unrelated changes? @oxsean please check https://github.com/apache/dubbo/pull/15338 -- 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: notifications-unsubscr...@dubbo.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: notifications-unsubscr...@dubbo.apache.org For additional commands, e-mail: notifications-h...@dubbo.apache.org
Re: [PR] fix: toCommaDelimitedString returns "null,null" when inputs are null [dubbo]
ankitshokeen closed pull request #15322: fix: toCommaDelimitedString returns "null,null" when inputs are null URL: https://github.com/apache/dubbo/pull/15322 -- 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: notifications-unsubscr...@dubbo.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: notifications-unsubscr...@dubbo.apache.org For additional commands, e-mail: notifications-h...@dubbo.apache.org
Re: [PR] fix: toCommaDelimitedString returns "null,null" when inputs are null [dubbo]
ankitshokeen commented on PR #15322: URL: https://github.com/apache/dubbo/pull/15322#issuecomment-2829899862 > Why are there so many unrelated changes? Invalid PR, please ignore working on another. -- 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: notifications-unsubscr...@dubbo.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: notifications-unsubscr...@dubbo.apache.org For additional commands, e-mail: notifications-h...@dubbo.apache.org
Re: [PR] fix: toCommaDelimitedString returns "null,null" when inputs are null [dubbo]
oxsean commented on PR #15322: URL: https://github.com/apache/dubbo/pull/15322#issuecomment-2826712210 Why are there so many unrelated changes? -- 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: notifications-unsubscr...@dubbo.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: notifications-unsubscr...@dubbo.apache.org For additional commands, e-mail: notifications-h...@dubbo.apache.org
Re: [PR] fix: toCommaDelimitedString returns "null,null" when inputs are null [dubbo]
ankitshokeen commented on PR #15322: URL: https://github.com/apache/dubbo/pull/15322#issuecomment-2818005991 @AlbumenJ @songxiaosheng @JunJieLiu51520 please review this PR -- 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: notifications-unsubscr...@dubbo.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: notifications-unsubscr...@dubbo.apache.org For additional commands, e-mail: notifications-h...@dubbo.apache.org
[PR] fix: toCommaDelimitedString returns "null,null" when inputs are null [dubbo]
ankitshokeen opened a new pull request, #15322: URL: https://github.com/apache/dubbo/pull/15322 ## What is the purpose of the change? This change corrects the behavior of StringUtils.toCommaDelimitedString(String one, String... others) to return null when either argument is null, as described in its Javadoc. Previously, the method would return a string like "null,null", which could lead to misleading outputs and test failures. This update ensures consistency between the method's implementation and its documented contract. ## Checklist - [x] Make sure there is a [GitHub_issue](https://github.com/apache/dubbo/issues) field for the change. - [x] Write a pull request description that is detailed enough to understand what the pull request does, how, and why. - [x] Write necessary unit-test to verify your logic correction. If the new feature or significant change is committed, please remember to add sample in [dubbo samples](https://github.com/apache/dubbo-samples) project. - [x] Make sure gitHub actions can pass. [Why the workflow is failing and how to fix it?](../CONTRIBUTING.md) -- 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: notifications-unsubscr...@dubbo.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: notifications-unsubscr...@dubbo.apache.org For additional commands, e-mail: notifications-h...@dubbo.apache.org
Re: [PR] fix: toCommaDelimitedString returns "null,null" when inputs are null [dubbo]
codecov-commenter commented on PR #15322: URL: https://github.com/apache/dubbo/pull/15322#issuecomment-2804949271 ## [Codecov](https://app.codecov.io/gh/apache/dubbo/pull/15322?dropdown=coverage&src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) Report Attention: Patch coverage is `68.53933%` with `28 lines` in your changes missing coverage. Please review. > Project coverage is 29.54%. Comparing base [(`731612b`)](https://app.codecov.io/gh/apache/dubbo/commit/731612bfef1332b7fa9c305cbfa9dd52df9dc30a?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) to head [(`c4d974d`)](https://app.codecov.io/gh/apache/dubbo/commit/c4d974d48d29281ad484a948c0330304a2dc59f9?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache). > Report is 11 commits behind head on 3.3. | [Files with missing lines](https://app.codecov.io/gh/apache/dubbo/pull/15322?dropdown=coverage&src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Patch % | Lines | |---|---|---| | [...stry/client/ServiceDiscoveryRegistryDirectory.java](https://app.codecov.io/gh/apache/dubbo/pull/15322?src=pr&el=tree&filepath=dubbo-registry%2Fdubbo-registry-api%2Fsrc%2Fmain%2Fjava%2Forg%2Fapache%2Fdubbo%2Fregistry%2Fclient%2FServiceDiscoveryRegistryDirectory.java&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-ZHViYm8tcmVnaXN0cnkvZHViYm8tcmVnaXN0cnktYXBpL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9kdWJiby9yZWdpc3RyeS9jbGllbnQvU2VydmljZURpc2NvdmVyeVJlZ2lzdHJ5RGlyZWN0b3J5LmphdmE=) | 70.21% | [10 Missing and 4 partials :warning: ](https://app.codecov.io/gh/apache/dubbo/pull/15322?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | | [...moting/transport/netty4/NettyConnectionClient.java](https://app.codecov.io/gh/apache/dubbo/pull/15322?src=pr&el=tree&filepath=dubbo-remoting%2Fdubbo-remoting-netty4%2Fsrc%2Fmain%2Fjava%2Forg%2Fapache%2Fdubbo%2Fremoting%2Ftransport%2Fnetty4%2FNettyConnectionClient.java&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-ZHViYm8tcmVtb3RpbmcvZHViYm8tcmVtb3RpbmctbmV0dHk0L3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9kdWJiby9yZW1vdGluZy90cmFuc3BvcnQvbmV0dHk0L05ldHR5Q29ubmVjdGlvbkNsaWVudC5qYXZh) | 40.00% | [8 Missing and 1 partial :warning: ](https://app.codecov.io/gh/apache/dubbo/pull/15322?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | | [.../dubbo/registry/integration/RegistryDirectory.java](https://app.codecov.io/gh/apache/dubbo/pull/15322?src=pr&el=tree&filepath=dubbo-registry%2Fdubbo-registry-api%2Fsrc%2Fmain%2Fjava%2Forg%2Fapache%2Fdubbo%2Fregistry%2Fintegration%2FRegistryDirectory.java&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-ZHViYm8tcmVnaXN0cnkvZHViYm8tcmVnaXN0cnktYXBpL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9kdWJiby9yZWdpc3RyeS9pbnRlZ3JhdGlvbi9SZWdpc3RyeURpcmVjdG9yeS5qYXZh) | 88.00% | [1 Missing and 2 partials :warning: ](https://app.codecov.io/gh/apache/dubbo/pull/15322?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | | [...ava/org/apache/dubbo/common/utils/StringUtils.java](https://app.codecov.io/gh/apache/dubbo/pull/15322?src=pr&el=tree&filepath=dubbo-common%2Fsrc%2Fmain%2Fjava%2Forg%2Fapache%2Fdubbo%2Fcommon%2Futils%2FStringUtils.java&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-ZHViYm8tY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9kdWJiby9jb21tb24vdXRpbHMvU3RyaW5nVXRpbHMuamF2YQ==) | 0.00% | [2 Missing :warning: ](https://app.codecov.io/gh/apache/dubbo/pull/15322?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | > :exclamation: There is a different number of reports uploaded between BASE (731612b) and HEAD (c4d974d). Click for more details. > > HEAD has 2 uploads less than BASE > >| Flag | BASE (731612b) | HEAD (c4d974d) | >|--|--|--| >|integration-tests|1|0| >|unit-tests|1|0| > Additional details and impacted files ```diff @@ Coverage Diff @@ ##3.3 #15322 +/- ## = - Coverage 60.75% 29.54% -31.22% + Complexity10917 8980 -1937 = Files 1886 1886 Lines 8609786165 +68 Branches 1289612909 +13 ==
