You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@bookkeeper.apache.org by sigee <gi...@git.apache.org> on 2017/06/29 13:51:02 UTC
[GitHub] bookkeeper pull request #217: Code cleanups
GitHub user sigee opened a pull request:
https://github.com/apache/bookkeeper/pull/217
Code cleanups
Descriptions of the changes in this PR:
There are some different kind of cleanups by commits. To make the code more readeable and improve code quality and improve performance in some cases.
E.g.:
- Remove unnecessary semicolons
- Remove unused/unneeded/duplicated imports
- Replace l to L in long literals
- Remove boxing on an already boxed value
- Replace iterations to bulk operations
---
Be sure to do all of the following to help us incorporate your contribution
quickly and easily:
- [ ] Make sure the PR title is formatted like:
`<Issue # or BOOKKEEPER-#>: Description of pull request`
`e.g. Issue 123: Description ...`
`e.g. BOOKKEEPER-1234: Description ...`
- [ ] Make sure tests pass via `mvn clean apache-rat:check install findbugs:check`.
- [ ] Replace `<Issue # or BOOKKEEPER-#>` in the title with the actual Issue/JIRA number.
---
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/sigee/bookkeeper cleanups
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/bookkeeper/pull/217.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 #217
----
commit 94741df4ffbe884b70ec31188c77d46750f8378b
Author: sigee <si...@gmail.com>
Date: 2017-06-29T09:07:51Z
Remove unnecessary semicolons
commit 99df3dc8f4182756f57e7ca07003e68e8b329f95
Author: sigee <si...@gmail.com>
Date: 2017-06-29T09:15:24Z
Symplify if statements
commit 88da9fe7dc9b6e2682d92dc9bf39e5cf4693376e
Author: sigee <si...@gmail.com>
Date: 2017-06-29T10:23:31Z
Use static member statically
commit 2bb697c53ee6dab82dfff03b7f24bac4c0c12ae0
Author: sigee <si...@gmail.com>
Date: 2017-06-29T12:52:20Z
Remove unused/unneeded/duplicated imports
commit bbb957621fd1a6939a9925a431901e9b8e15efdd
Author: sigee <si...@gmail.com>
Date: 2017-06-29T13:09:55Z
Replace l to L in long literals
commit 507a1ae4ba28b7bca324a9317cc0880f021b6280
Author: sigee <si...@gmail.com>
Date: 2017-06-29T13:11:41Z
Remove boxing on an already boxed value
commit 12c96488141ddd22afb5f4deb1394cf7373c1fcb
Author: sigee <si...@gmail.com>
Date: 2017-06-29T13:14:30Z
Replace manual array to collection to Collection.addAll()
commit 41c18840b5236b1bd2e9dccbaf654df37b1b6ff8
Author: sigee <si...@gmail.com>
Date: 2017-06-29T13:17:52Z
Remove redundant String constructor call
commit 6934fabee27beaf00876f9884a2a5344ecaa4ee3
Author: sigee <si...@gmail.com>
Date: 2017-06-29T13:21:40Z
Replace iterations to bulk operations
----
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] bookkeeper issue #217: Code cleanups
Posted by sigee <gi...@git.apache.org>.
Github user sigee commented on the issue:
https://github.com/apache/bookkeeper/pull/217
@eolivelli,
If it is fine for you I can keep this PR up to date after every merge you make (and break this one).
So you can merge it when you want. ;)
I can add maven checkstyle plugin and attache it to the validate goal if you would like.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] bookkeeper issue #217: Code cleanups
Posted by eolivelli <gi...@git.apache.org>.
Github user eolivelli commented on the issue:
https://github.com/apache/bookkeeper/pull/217
@sigee I will be happy to have the checkstyle plugin on but we should discuss on the dev@bookeeper.apache.org list about the rules to set up, maybe you can start a new thread
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] bookkeeper issue #217: Code cleanups
Posted by eolivelli <gi...@git.apache.org>.
Github user eolivelli commented on the issue:
https://github.com/apache/bookkeeper/pull/217
Thank you @sigee interesting proposal.
@sijie I think this patch could break some pending Pull Request
in general I am OK with this kind of patch but I think it would be better to have such cleanup at the start of next release or at the end of 4.5
it would be great to add maven checkstyle plugin to in the build pipeline in order to guarantee our coding conventions
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] bookkeeper issue #217: Code cleanups
Posted by jiazhai <gi...@git.apache.org>.
Github user jiazhai commented on the issue:
https://github.com/apache/bookkeeper/pull/217
The change looks good, Please open an issue or a Jira ticket for this PR, and at the end of this PR description, For the checks that you have done, please place "x" in the "[ ]".
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] bookkeeper issue #217: Code cleanups
Posted by sijie <gi...@git.apache.org>.
Github user sijie commented on the issue:
https://github.com/apache/bookkeeper/pull/217
I am okay with using a checkstyle plugin. it shouldn't be a big deal with existing pull requests.
I am fine with merging this change and having a separate follow up jira to enable checkstyle plugin. Since DistributedLog is graduated as a subproject of BookKeeper, and there is already checkstyle rules in DistributedLog, it would be good to reuse the checkstyle rules there. https://github.com/apache/incubator-distributedlog/blob/master/distributedlog-build-tools/src/main/resources/distributedlog/checkstyle.xml
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---