You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@jmeter.apache.org by ham1 <gi...@git.apache.org> on 2017/11/09 22:27:16 UTC
[GitHub] jmeter pull request #324: Save backup refactor
GitHub user ham1 opened a pull request:
https://github.com/apache/jmeter/pull/324
Save backup refactor
Further refactoring for readability to `createBackupFile` and other minor improvements.
## Description
Refactoring `createBackupFile` and minor tidy up of surrounding code.
I've got a bit carried away but tried to commit often so that at least some of the changes can be easily reviewed and applied.
## Motivation and Context
The aim is to improve readability and thus make fixing, testing and improving easier.
## How Has This Been Tested?
Manually with both backup settings set. Unit testing is still not very easy.
## Screenshots (if appropriate):
## Types of changes
<!--- What types of changes does your code introduce? Put an `x` in all the boxes that apply: -->
- [x] Code improvement
- [ ] Bug fix (non-breaking change which fixes an issue)
- [ ] New feature (non-breaking change which adds functionality)
- [ ] Breaking change (fix or feature that would cause existing functionality to not work as expected)
## Checklist:
<!--- Go over all the following points, and put an `x` in all the boxes that apply. -->
<!--- If you're unsure about any of these, don't hesitate to ask. We're here to help! -->
- [x] My code follows the code style of this project.
- [ ] My change requires a change to the documentation.
- [ ] I have updated the documentation accordingly.
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/ham1/jmeter save_backup_refactor
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/jmeter/pull/324.patch
To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:
This closes #324
----
commit ccf7d0b503c0f1fc53952d36e1e01b9cec089aa4
Author: Graham Russell <gr...@ham1.co.uk>
Date: 2017-11-09T12:26:07Z
removed unused import
commit 0b1c073a17df6081c87dbc09914345d9a414a987
Author: Graham Russell <gr...@ham1.co.uk>
Date: 2017-11-09T12:27:29Z
refactored checkAcceptableForTestFragment for readability
commit 29ac2638fc93c3536b79c9debc8b7cd777d75dc1
Author: Graham Russell <gr...@ham1.co.uk>
Date: 2017-11-09T12:31:30Z
whitespace, strings and comment tidy up
commit 4e13847b4008b6b3b93873107411f005ab7db0e2
Author: Graham Russell <gr...@ham1.co.uk>
Date: 2017-11-09T15:21:58Z
utilised LastModifiedFileComparator
commit 1f4dd8d337fa7532532e4dd8123cc217081a2472
Author: Graham Russell <gr...@ham1.co.uk>
Date: 2017-11-09T16:43:51Z
whitespace etc for readability
commit 863f3566d293f196ed75d8057de93febe85e5e1f
Author: Graham Russell <gr...@ham1.co.uk>
Date: 2017-11-09T16:48:05Z
Moved code to deal with files to delete to separate method
commit a7523a178f84293dbb871cedf74e73390e1a9ff7
Author: Graham Russell <gr...@ham1.co.uk>
Date: 2017-11-09T17:29:45Z
Removed redundant try, catch and use more concise syntax
commit 9eeac9070229eaead36ede36e7d9e5e72f1b82ad
Author: Graham Russell <gr...@ham1.co.uk>
Date: 2017-11-09T19:40:17Z
removed predicate method
commit 487cc43ec66d492ab32f157a9fb0fc1c2e64aa5c
Author: Graham Russell <gr...@ham1.co.uk>
Date: 2017-11-09T19:40:52Z
further decomposed backupFilesToDelete
----
---
[GitHub] jmeter issue #324: Save backup refactor
Posted by codecov-io <gi...@git.apache.org>.
Github user codecov-io commented on the issue:
https://github.com/apache/jmeter/pull/324
# [Codecov](https://codecov.io/gh/apache/jmeter/pull/324?src=pr&el=h1) Report
> Merging [#324](https://codecov.io/gh/apache/jmeter/pull/324?src=pr&el=desc) into [trunk](https://codecov.io/gh/apache/jmeter/commit/20e83927fc641c9692f9ea08e0b6f6a42a4c2c9f?src=pr&el=desc) will **increase** coverage by `<.01%`.
> The diff coverage is `0%`.
[](https://codecov.io/gh/apache/jmeter/pull/324?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## trunk #324 +/- ##
============================================
+ Coverage 57.75% 57.75% +<.01%
- Complexity 9913 9915 +2
============================================
Files 1139 1139
Lines 73051 73050 -1
Branches 7303 7300 -3
============================================
+ Hits 42187 42188 +1
+ Misses 28392 28390 -2
Partials 2472 2472
```
| [Impacted Files](https://codecov.io/gh/apache/jmeter/pull/324?src=pr&el=tree) | Coverage Δ | Complexity Δ | |
|---|---|---|---|
| [src/core/org/apache/jmeter/gui/action/Save.java](https://codecov.io/gh/apache/jmeter/pull/324?src=pr&el=tree#diff-c3JjL2NvcmUvb3JnL2FwYWNoZS9qbWV0ZXIvZ3VpL2FjdGlvbi9TYXZlLmphdmE=) | `13.25% <0%> (+0.07%)` | `4 <0> (ø)` | :arrow_down: |
| [...ocol/jms/org/apache/jmeter/protocol/jms/Utils.java](https://codecov.io/gh/apache/jmeter/pull/324?src=pr&el=tree#diff-c3JjL3Byb3RvY29sL2ptcy9vcmcvYXBhY2hlL2ptZXRlci9wcm90b2NvbC9qbXMvVXRpbHMuamF2YQ==) | `54.83% <0%> (-2.16%)` | `15% <0%> (ø)` | |
| [...apache/jmeter/extractor/TestBoundaryExtractor.java](https://codecov.io/gh/apache/jmeter/pull/324?src=pr&el=tree#diff-dGVzdC9zcmMvb3JnL2FwYWNoZS9qbWV0ZXIvZXh0cmFjdG9yL1Rlc3RCb3VuZGFyeUV4dHJhY3Rvci5qYXZh) | `97.88% <0%> (ø)` | `17% <0%> (ø)` | :arrow_down: |
| [...c/core/org/apache/jmeter/reporters/Summariser.java](https://codecov.io/gh/apache/jmeter/pull/324?src=pr&el=tree#diff-c3JjL2NvcmUvb3JnL2FwYWNoZS9qbWV0ZXIvcmVwb3J0ZXJzL1N1bW1hcmlzZXIuamF2YQ==) | `86.15% <0%> (+0.76%)` | `19% <0%> (+1%)` | :arrow_up: |
| [...s/org/apache/jmeter/timers/PoissonRandomTimer.java](https://codecov.io/gh/apache/jmeter/pull/324?src=pr&el=tree#diff-c3JjL2NvbXBvbmVudHMvb3JnL2FwYWNoZS9qbWV0ZXIvdGltZXJzL1BvaXNzb25SYW5kb21UaW1lci5qYXZh) | `78.37% <0%> (+5.4%)` | `10% <0%> (+1%)` | :arrow_up: |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/jmeter/pull/324?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/jmeter/pull/324?src=pr&el=footer). Last update [20e8392...487cc43](https://codecov.io/gh/apache/jmeter/pull/324?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
---
[GitHub] jmeter issue #324: Save backup refactor
Posted by ham1 <gi...@git.apache.org>.
Github user ham1 commented on the issue:
https://github.com/apache/jmeter/pull/324
@pmouawad Thank you for committing my changes - in future I'm happy to rebase and fix merge issues so you don't have to :)
There were only a few minor things:
- formatting of streams api code - they seem to be mostly on one line, whereas I think they read much better when split, (perhaps this is the eclipse formatter?)
- some extra whitespace was added (maybe eclipse settings?)
- I question the JavaDoc on the private methods - it didn't seem to help, I had hoped the method name was sufficient - is there a reason it was added?
- formatting of JavaDoc comments, I think we should agree on a format and make sure it's updated in the style guide on the wiki as we currently have a mix and it would be good to agree to prevent further inconsistency (the location of the description of the param, same or new line)
What's the best way for me to provide these changes? Force push to this PR, new PR, email patch file?
Thanks
---
[GitHub] jmeter pull request #324: Save backup refactor
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/jmeter/pull/324
---
[GitHub] jmeter issue #324: Save backup refactor
Posted by pmouawad <gi...@git.apache.org>.
Github user pmouawad commented on the issue:
https://github.com/apache/jmeter/pull/324
Thanks @ham1 ,
Could you review my commit as your PR had a conflict after merging of Workbench before it.
I think I didn't miss anything but another eye is welcome.
Regards
---