[GitHub] [skywalking] wu-sheng commented on issue #4264: Refactor SnifferConfigInitializer and related componets with new features in JDK8+
wu-sheng commented on issue #4264: Refactor SnifferConfigInitializer and related componets with new features in JDK8+ URL: https://github.com/apache/skywalking/pull/4264#issuecomment-576015199 There is already a `codeStyle.xml` in root path, once, you import it, you are good. @kezhenxu94 's point is making it better. The format I pointed out, is not about that part. 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [skywalking] wu-sheng commented on issue #4264: Refactor SnifferConfigInitializer and related componets with new features in JDK8+
wu-sheng commented on issue #4264: Refactor SnifferConfigInitializer and related componets with new features in JDK8+ URL: https://github.com/apache/skywalking/pull/4264#issuecomment-576013141 @JohnNiang Why do you close? I am a little confused. 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [skywalking] wu-sheng commented on issue #4264: Refactor SnifferConfigInitializer and related componets with new features in JDK8+
wu-sheng commented on issue #4264: Refactor SnifferConfigInitializer and related componets with new features in JDK8+ URL: https://github.com/apache/skywalking/pull/4264#issuecomment-575960326 > Yeah, this PR doesn't contain any new feature(for skywalking), just a refactor with the new feature of JDK8+. (BTW, I have never said this PR contains any new feature. : P I am not saying about the purpose of this PR. I am still talking about the `Blank` thing. I still think we don't need that. This util is also used in the backend, check the package, it doesn't belong to the agent. 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [skywalking] wu-sheng commented on issue #4264: Refactor SnifferConfigInitializer and related componets with new features in JDK8+
wu-sheng commented on issue #4264: Refactor SnifferConfigInitializer and related componets with new features in JDK8+ URL: https://github.com/apache/skywalking/pull/4264#issuecomment-575953773 > Empty means that the string contains nothing or is null, while Blank is not only Empty string, but also contains only whitespace character(s). I think @kezhenxu94 's point is why, I have the same concern too. Why do we need to separate these two? First, the `Blank` cost more resources, what is the benefit? Then, allowing these two existing will make the further reviews(of other PRs) confusing. It is hard to tell from all reviewers perspective, which one should be used in which cases. If there isn't a case, required this, which I think there isn't as this PR didn't add a new feature, but refactor, I hope we don't add these new APIs. 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [skywalking] wu-sheng commented on issue #4264: Refactor SnifferConfigInitializer and related componets with new features in JDK8+
wu-sheng commented on issue #4264: Refactor SnifferConfigInitializer and related componets with new features in JDK8+ URL: https://github.com/apache/skywalking/pull/4264#issuecomment-575865246 > we may have to differentiate the meaning between Empty and Blank What is the difference? And what is your case? 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [skywalking] wu-sheng commented on issue #4264: Refactor SnifferConfigInitializer and related componets with new features in JDK8+
wu-sheng commented on issue #4264: Refactor SnifferConfigInitializer and related componets with new features in JDK8+ URL: https://github.com/apache/skywalking/pull/4264#issuecomment-575849109 CI fails, please fix those. 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
