You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@hc.apache.org by GitBox <gi...@apache.org> on 2021/03/01 05:29:05 UTC
[GitHub] [httpcomponents-core] arturobernalg opened a new pull request #271: Feature/pmd warnings
arturobernalg opened a new pull request #271:
URL: https://github.com/apache/httpcomponents-core/pull/271
The idea it's fix some of the PMD warnings such :
- Change Switch statements for "if"
- Use String constant
- Add serialVersionUID
- Use Files.newInputStream
- Use empty array
----------------------------------------------------------------
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:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@hc.apache.org
For additional commands, e-mail: dev-help@hc.apache.org
[GitHub] [httpcomponents-core] ok2c commented on a change in pull request #271: feature/pmd warnings
Posted by GitBox <gi...@apache.org>.
ok2c commented on a change in pull request #271:
URL: https://github.com/apache/httpcomponents-core/pull/271#discussion_r584519840
##########
File path: httpcore5-h2/src/main/java/org/apache/hc/core5/http2/impl/nio/ClientH2StreamHandler.java
##########
@@ -166,21 +166,18 @@ private void commitRequest(final HttpRequest request, final EntityDetails entity
@Override
public void produceOutput() throws HttpException, IOException {
- switch (requestState) {
- case HEADERS:
- exchangeHandler.produceRequest(new RequestChannel() {
+ if (requestState == MessageState.HEADERS) {
Review comment:
@arturobernalg Why? What is exactly wrong with `switch case` here? How is this an improvement?
----------------------------------------------------------------
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:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@hc.apache.org
For additional commands, e-mail: dev-help@hc.apache.org
[GitHub] [httpcomponents-core] arturobernalg commented on a change in pull request #271: feature/pmd warnings
Posted by GitBox <gi...@apache.org>.
arturobernalg commented on a change in pull request #271:
URL: https://github.com/apache/httpcomponents-core/pull/271#discussion_r584613033
##########
File path: httpcore5-h2/src/main/java/org/apache/hc/core5/http2/frame/DefaultFrameFactory.java
##########
@@ -39,31 +39,35 @@
public class DefaultFrameFactory extends FrameFactory {
public static final FrameFactory INSTANCE = new DefaultFrameFactory();
+ /**
+ * A String for a {@code "Stream id"}.
+ */
+ private static final String STREAM_ID = "Stream id";
@Override
public RawFrame createHeaders(final int streamId, final ByteBuffer payload, final boolean endHeaders, final boolean endStream) {
- Args.positive(streamId, "Stream id");
+ Args.positive(streamId, STREAM_ID);
Review comment:
Hi @ok2c
it's one of the sonar error-prone rules to make easy the refactor process.
https://rules.sonarsource.com/java/RSPEC-1192
----------------------------------------------------------------
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:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@hc.apache.org
For additional commands, e-mail: dev-help@hc.apache.org
[GitHub] [httpcomponents-core] arturobernalg commented on pull request #271: feature/pmd warnings
Posted by GitBox <gi...@apache.org>.
arturobernalg commented on pull request #271:
URL: https://github.com/apache/httpcomponents-core/pull/271#issuecomment-788062265
Hi @ok2c
I'll do. I'm agree that some of the change are subjective.
Regards,
----------------------------------------------------------------
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:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@hc.apache.org
For additional commands, e-mail: dev-help@hc.apache.org
[GitHub] [httpcomponents-core] ok2c commented on pull request #271: feature/pmd warnings
Posted by GitBox <gi...@apache.org>.
ok2c commented on pull request #271:
URL: https://github.com/apache/httpcomponents-core/pull/271#issuecomment-787867060
@arturobernalg Overall this whole change-set makes no sense unless you also introduce PMC checks into the project build.
----------------------------------------------------------------
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:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@hc.apache.org
For additional commands, e-mail: dev-help@hc.apache.org
[GitHub] [httpcomponents-core] arturobernalg closed pull request #271: feature/pmd warnings
Posted by GitBox <gi...@apache.org>.
arturobernalg closed pull request #271:
URL: https://github.com/apache/httpcomponents-core/pull/271
----------------------------------------------------------------
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:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@hc.apache.org
For additional commands, e-mail: dev-help@hc.apache.org
[GitHub] [httpcomponents-core] ok2c commented on pull request #271: feature/pmd warnings
Posted by GitBox <gi...@apache.org>.
ok2c commented on pull request #271:
URL: https://github.com/apache/httpcomponents-core/pull/271#issuecomment-787999931
@arturobernalg One thing I should have said as well: If you want to contribute to the project there are better ways to do so than adding PMD checks. Talk to us on `dev@hc.apache.org`.
----------------------------------------------------------------
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:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@hc.apache.org
For additional commands, e-mail: dev-help@hc.apache.org
[GitHub] [httpcomponents-core] ok2c commented on a change in pull request #271: feature/pmd warnings
Posted by GitBox <gi...@apache.org>.
ok2c commented on a change in pull request #271:
URL: https://github.com/apache/httpcomponents-core/pull/271#discussion_r584517300
##########
File path: httpcore5-h2/src/main/java/org/apache/hc/core5/http2/frame/DefaultFrameFactory.java
##########
@@ -39,31 +39,35 @@
public class DefaultFrameFactory extends FrameFactory {
public static final FrameFactory INSTANCE = new DefaultFrameFactory();
+ /**
+ * A String for a {@code "Stream id"}.
+ */
+ private static final String STREAM_ID = "Stream id";
@Override
public RawFrame createHeaders(final int streamId, final ByteBuffer payload, final boolean endHeaders, final boolean endStream) {
- Args.positive(streamId, "Stream id");
+ Args.positive(streamId, STREAM_ID);
Review comment:
@arturobernalg Why? What is exactly the benefit of doing so?
----------------------------------------------------------------
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:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@hc.apache.org
For additional commands, e-mail: dev-help@hc.apache.org
[GitHub] [httpcomponents-core] arturobernalg commented on a change in pull request #271: feature/pmd warnings
Posted by GitBox <gi...@apache.org>.
arturobernalg commented on a change in pull request #271:
URL: https://github.com/apache/httpcomponents-core/pull/271#discussion_r584611300
##########
File path: httpcore5-h2/src/main/java/org/apache/hc/core5/http2/impl/nio/ClientH2StreamHandler.java
##########
@@ -166,21 +166,18 @@ private void commitRequest(final HttpRequest request, final EntityDetails entity
@Override
public void produceOutput() throws HttpException, IOException {
- switch (requestState) {
- case HEADERS:
- exchangeHandler.produceRequest(new RequestChannel() {
+ if (requestState == MessageState.HEADERS) {
Review comment:
Hi @ok2c.
IMO Increase the code readability. If you notice I only change those statement with only one or two conditions.
----------------------------------------------------------------
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:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@hc.apache.org
For additional commands, e-mail: dev-help@hc.apache.org
[GitHub] [httpcomponents-core] ok2c commented on a change in pull request #271: feature/pmd warnings
Posted by GitBox <gi...@apache.org>.
ok2c commented on a change in pull request #271:
URL: https://github.com/apache/httpcomponents-core/pull/271#discussion_r584626964
##########
File path: httpcore5-h2/src/main/java/org/apache/hc/core5/http2/impl/nio/ClientH2StreamHandler.java
##########
@@ -166,21 +166,18 @@ private void commitRequest(final HttpRequest request, final EntityDetails entity
@Override
public void produceOutput() throws HttpException, IOException {
- switch (requestState) {
- case HEADERS:
- exchangeHandler.produceRequest(new RequestChannel() {
+ if (requestState == MessageState.HEADERS) {
Review comment:
@arturobernalg I do not see exactly how this makes anything better readable. This is _very_ subjective and purely driven by personal fancy of Sonar developers.
----------------------------------------------------------------
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:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@hc.apache.org
For additional commands, e-mail: dev-help@hc.apache.org
[GitHub] [httpcomponents-core] ok2c edited a comment on pull request #271: feature/pmd warnings
Posted by GitBox <gi...@apache.org>.
ok2c edited a comment on pull request #271:
URL: https://github.com/apache/httpcomponents-core/pull/271#issuecomment-787867060
@arturobernalg Overall this whole change-set makes no sense unless you also introduce PMD checks into the project build.
----------------------------------------------------------------
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:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@hc.apache.org
For additional commands, e-mail: dev-help@hc.apache.org
[GitHub] [httpcomponents-core] ok2c commented on a change in pull request #271: feature/pmd warnings
Posted by GitBox <gi...@apache.org>.
ok2c commented on a change in pull request #271:
URL: https://github.com/apache/httpcomponents-core/pull/271#discussion_r584619607
##########
File path: httpcore5-h2/src/main/java/org/apache/hc/core5/http2/frame/DefaultFrameFactory.java
##########
@@ -39,31 +39,35 @@
public class DefaultFrameFactory extends FrameFactory {
public static final FrameFactory INSTANCE = new DefaultFrameFactory();
+ /**
+ * A String for a {@code "Stream id"}.
+ */
+ private static final String STREAM_ID = "Stream id";
@Override
public RawFrame createHeaders(final int streamId, final ByteBuffer payload, final boolean endHeaders, final boolean endStream) {
- Args.positive(streamId, "Stream id");
+ Args.positive(streamId, STREAM_ID);
Review comment:
@arturobernalg I disagree. Sonar does a lot of questionable stuff and I do not see how this would simplify refactoring process at all.
----------------------------------------------------------------
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:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@hc.apache.org
For additional commands, e-mail: dev-help@hc.apache.org