New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Checkstyle: LineLength max 165, AnonInnerLength 45 and other whitespace checks #358

Closed
wants to merge 1 commit into
base: trunk
from

Conversation

Projects
None yet
3 participants
@ham1

ham1 commented Dec 12, 2017

Also removed some unnecessary StringBuilders

How Has This Been Tested?

ant checkstyle test

@codecov-io

This comment has been minimized.

Show comment
Hide comment
@codecov-io

codecov-io Dec 12, 2017

Codecov Report

Merging #358 into trunk will decrease coverage by <.01%.
The diff coverage is 40.92%.

Impacted file tree graph

@@             Coverage Diff              @@
##              trunk     #358      +/-   ##
============================================
- Coverage     58.37%   58.37%   -0.01%     
- Complexity    10316    10317       +1     
============================================
  Files          1170     1170              
  Lines         74347    74411      +64     
  Branches       7324     7326       +2     
============================================
+ Hits          43399    43435      +36     
- Misses        28421    28451      +30     
+ Partials       2527     2525       -2
Impacted Files Coverage Δ Complexity Δ
.../org/apache/jmeter/report/core/SampleMetadata.java 26.08% <ø> (ø) 8 <0> (ø) ⬇️
...jmeter/protocol/http/util/accesslog/LogFilter.java 81.3% <ø> (ø) 35 <0> (ø) ⬇️
...protocol/http/sampler/LazySchemeSocketFactory.java 66.66% <ø> (ø) 5 <0> (ø) ⬇️
...ter/extractor/json/jsonpath/JSONPostProcessor.java 66.95% <0%> (ø) 28 <0> (ø) ⬇️
...jmeter/visualizers/backend/BackendListenerGui.java 80.37% <0%> (ø) 13 <0> (ø) ⬇️
.../org/apache/jmeter/engine/util/FunctionParser.java 69.35% <0%> (ø) 33 <0> (ø) ⬇️
...he/jmeter/visualizers/backend/BackendListener.java 10% <0%> (-0.29%) 7 <0> (ø)
...er/protocol/http/proxy/AbstractSamplerCreator.java 60% <0%> (+0.62%) 6 <1> (ø) ⬇️
...e/org/apache/jmeter/gui/action/OpenLinkAction.java 0% <0%> (ø) 0 <0> (ø) ⬇️
...apache/jmeter/control/gui/ModuleControllerGui.java 31.25% <0%> (ø) 8 <0> (ø) ⬇️
... and 37 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 33f09a9...7535a0c. Read the comment docs.

codecov-io commented Dec 12, 2017

Codecov Report

Merging #358 into trunk will decrease coverage by <.01%.
The diff coverage is 40.92%.

Impacted file tree graph

@@             Coverage Diff              @@
##              trunk     #358      +/-   ##
============================================
- Coverage     58.37%   58.37%   -0.01%     
- Complexity    10316    10317       +1     
============================================
  Files          1170     1170              
  Lines         74347    74411      +64     
  Branches       7324     7326       +2     
============================================
+ Hits          43399    43435      +36     
- Misses        28421    28451      +30     
+ Partials       2527     2525       -2
Impacted Files Coverage Δ Complexity Δ
.../org/apache/jmeter/report/core/SampleMetadata.java 26.08% <ø> (ø) 8 <0> (ø) ⬇️
...jmeter/protocol/http/util/accesslog/LogFilter.java 81.3% <ø> (ø) 35 <0> (ø) ⬇️
...protocol/http/sampler/LazySchemeSocketFactory.java 66.66% <ø> (ø) 5 <0> (ø) ⬇️
...ter/extractor/json/jsonpath/JSONPostProcessor.java 66.95% <0%> (ø) 28 <0> (ø) ⬇️
...jmeter/visualizers/backend/BackendListenerGui.java 80.37% <0%> (ø) 13 <0> (ø) ⬇️
.../org/apache/jmeter/engine/util/FunctionParser.java 69.35% <0%> (ø) 33 <0> (ø) ⬇️
...he/jmeter/visualizers/backend/BackendListener.java 10% <0%> (-0.29%) 7 <0> (ø)
...er/protocol/http/proxy/AbstractSamplerCreator.java 60% <0%> (+0.62%) 6 <1> (ø) ⬇️
...e/org/apache/jmeter/gui/action/OpenLinkAction.java 0% <0%> (ø) 0 <0> (ø) ⬇️
...apache/jmeter/control/gui/ModuleControllerGui.java 31.25% <0%> (ø) 8 <0> (ø) ⬇️
... and 37 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 33f09a9...7535a0c. Read the comment docs.

@pmouawad

This comment has been minimized.

Show comment
Hide comment
@pmouawad

pmouawad Dec 20, 2017

Contributor

Hi @ham1 ,
Thanks for this PR.
Unfortunately I am not very comfortable with PR that touch a lot of file and where the code modification may touch more things than what the PR describes.

Would it be possible to rebase your PR and split it into at max 10 files ?
It is easier to review and merging is faster for me, which will mean I'll merge it earlier and you won't have to rebase.

Also if possible, try to make the PR really only touch what it pretends to.
Sonar fixes might sometimes break existing code so they need careful review and merge.
Thanks a lot again for all your work !

Regards

Contributor

pmouawad commented Dec 20, 2017

Hi @ham1 ,
Thanks for this PR.
Unfortunately I am not very comfortable with PR that touch a lot of file and where the code modification may touch more things than what the PR describes.

Would it be possible to rebase your PR and split it into at max 10 files ?
It is easier to review and merging is faster for me, which will mean I'll merge it earlier and you won't have to rebase.

Also if possible, try to make the PR really only touch what it pretends to.
Sonar fixes might sometimes break existing code so they need careful review and merge.
Thanks a lot again for all your work !

Regards

@ham1

This comment has been minimized.

Show comment
Hide comment
@ham1

ham1 Dec 21, 2017

Sorry, yeah I see how this is difficult to review. I tend to change things as I see them in the file I'm editing. I will revise and split it up into one type of change per commit, is that ok? It might be more than 10 files but should be easier to review.

ham1 commented Dec 21, 2017

Sorry, yeah I see how this is difficult to review. I tend to change things as I see them in the file I'm editing. I will revise and split it up into one type of change per commit, is that ok? It might be more than 10 files but should be easier to review.

@ham1 ham1 referenced this pull request Dec 23, 2017

Closed

Set max line length to 160 (eventual target 120). #366

2 of 2 tasks complete
Checkstyle: LineLength max 165, AnonInnerLength 45 and other whitespa…
…ce checks

Also removed some unnecessary StringBuilders

asfgit pushed a commit that referenced this pull request Feb 11, 2018

Correct typecast paren padding
Part of #358 on github. Based on contribution by Graham Russell (graham at ham1.co.uk)


git-svn-id: https://svn.apache.org/repos/asf/jmeter/trunk@1823897 13f79535-47bb-0310-9956-ffa450edef68

asfgit pushed a commit that referenced this pull request Feb 11, 2018

Correct method param pad
plus some other whitespace policing.
Part of #358 on github. Based on contribution by Graham Russell (graham at ham1.co.uk)


git-svn-id: https://svn.apache.org/repos/asf/jmeter/trunk@1823898 13f79535-47bb-0310-9956-ffa450edef68

asfgit pushed a commit that referenced this pull request Feb 11, 2018

Use empty for iterator pad
Part of #358 on github. Based on contribution by Graham Russell (graham at ham1.co.uk)


git-svn-id: https://svn.apache.org/repos/asf/jmeter/trunk@1823899 13f79535-47bb-0310-9956-ffa450edef68

asfgit pushed a commit that referenced this pull request Feb 11, 2018

Use empty for initializer pad
Part of #358 on github. Based on contribution by Graham Russell (graham at ham1.co.uk)


git-svn-id: https://svn.apache.org/repos/asf/jmeter/trunk@1823900 13f79535-47bb-0310-9956-ffa450edef68

asfgit pushed a commit that referenced this pull request Feb 11, 2018

Enable NoLineWrap
Part of #358 on github. Based on contribution by Graham Russell (graham at ham1.co.uk)


git-svn-id: https://svn.apache.org/repos/asf/jmeter/trunk@1823901 13f79535-47bb-0310-9956-ffa450edef68

asfgit pushed a commit that referenced this pull request Feb 11, 2018

Start with enforcing a maximum length of 200 chars
Part of #358 on github. Based on contribution by Graham Russell (graham at ham1.co.uk)


git-svn-id: https://svn.apache.org/repos/asf/jmeter/trunk@1823903 13f79535-47bb-0310-9956-ffa450edef68

asfgit pushed a commit that referenced this pull request Feb 11, 2018

Enforce max anon inner length of 45 lines
Part of #358 on github. Based on contribution by Graham Russell (graham at ham1.co.uk)


git-svn-id: https://svn.apache.org/repos/asf/jmeter/trunk@1823905 13f79535-47bb-0310-9956-ffa450edef68

asfgit pushed a commit that referenced this pull request Feb 11, 2018

Get down to a maximum length of 200 chars
Part of #358 on github. Based on contribution by Graham Russell (graham at ham1.co.uk)



git-svn-id: https://svn.apache.org/repos/asf/jmeter/trunk@1823906 13f79535-47bb-0310-9956-ffa450edef68

asfgit pushed a commit that referenced this pull request Feb 11, 2018

Get down to a maximum length of 175 chars
Part of #358 on github. Based on contribution by Graham Russell (graham at ham1.co.uk)


git-svn-id: https://svn.apache.org/repos/asf/jmeter/trunk@1823908 13f79535-47bb-0310-9956-ffa450edef68

asfgit pushed a commit that referenced this pull request Feb 12, 2018

Get down to a maximum length of 170 chars
Part of #358 on github. Based on contribution by Graham Russell (graham at ham1.co.uk)



git-svn-id: https://svn.apache.org/repos/asf/jmeter/trunk@1823932 13f79535-47bb-0310-9956-ffa450edef68

asfgit pushed a commit that referenced this pull request Feb 12, 2018

Get down to a maximum length of 165 chars
Part of #358 on github. Based on contribution by Graham Russell (graham at ham1.co.uk)



git-svn-id: https://svn.apache.org/repos/asf/jmeter/trunk@1823940 13f79535-47bb-0310-9956-ffa450edef68

asfgit pushed a commit that referenced this pull request Feb 12, 2018

Extract common code into a private helper.
Preparation to get line length down to 160 chars.
In preparation for #358 on github


git-svn-id: https://svn.apache.org/repos/asf/jmeter/trunk@1823944 13f79535-47bb-0310-9956-ffa450edef68

asfgit pushed a commit that referenced this pull request Feb 12, 2018

Refactor common code into private helper.
As a bonus re-format a small if clause.

Preparation for #358 on github.


git-svn-id: https://svn.apache.org/repos/asf/jmeter/trunk@1823947 13f79535-47bb-0310-9956-ffa450edef68

asfgit pushed a commit that referenced this pull request Feb 12, 2018

Refactor common code into private helper.
Preparation for #358 on github.



git-svn-id: https://svn.apache.org/repos/asf/jmeter/trunk@1823948 13f79535-47bb-0310-9956-ffa450edef68

asfgit pushed a commit that referenced this pull request Feb 12, 2018

Get down to a maximum length of 160 chars
Part of #358 on github. Based on contribution by Graham Russell (graham at ham1.co.uk)


git-svn-id: https://svn.apache.org/repos/asf/jmeter/trunk@1823949 13f79535-47bb-0310-9956-ffa450edef68

asfgit pushed a commit that referenced this pull request Feb 12, 2018

asfgit pushed a commit that referenced this pull request Feb 12, 2018

asfgit pushed a commit that referenced this pull request Feb 12, 2018

asfgit pushed a commit that referenced this pull request Feb 12, 2018

asfgit pushed a commit that referenced this pull request Feb 12, 2018

No need to explicitly create an array for the arguments.
This is the last part of the Checkstyle pull requests #358, #365, #366 and #375.
It closes #366 and #358 on github.

Based on contribution by Graham Russell (graham at ham1.co.uk) 


git-svn-id: https://svn.apache.org/repos/asf/jmeter/trunk@1823957 13f79535-47bb-0310-9956-ffa450edef68

@asfgit asfgit closed this in 80c12b6 Feb 12, 2018

@ham1 ham1 deleted the ham1:more_checkstyle branch Feb 12, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment