You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@accumulo.apache.org by GitBox <gi...@apache.org> on 2020/03/04 21:20:35 UTC
[GitHub] [accumulo] jmark99 opened a new pull request #1546: Replace
try/finally with try-with-resources.
jmark99 opened a new pull request #1546: Replace try/finally with try-with-resources.
URL: https://github.com/apache/accumulo/pull/1546
Implement the try-with-resources construct available in Java 7+ where applicable.
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [accumulo] ctubbsii commented on a change in pull request #1546:
Replace try/finally with try-with-resources.
Posted by GitBox <gi...@apache.org>.
ctubbsii commented on a change in pull request #1546: Replace try/finally with try-with-resources.
URL: https://github.com/apache/accumulo/pull/1546#discussion_r387942650
##########
File path: core/src/main/java/org/apache/accumulo/core/cli/ClientOpts.java
##########
@@ -85,16 +85,10 @@ String process(String value) {
justification = "app is run in same security context as user providing the filename")
@Override
String process(String value) {
- Scanner scanner = null;
- try {
- scanner = new Scanner(new File(value), UTF_8);
+ try (Scanner scanner = new Scanner(new File(value), UTF_8)) {
Review comment:
This is a good opportunity to use `var`:
```suggestion
try (var scanner = new Scanner(new File(value), UTF_8)) {
```
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [accumulo] ctubbsii merged pull request #1546: Replace try/finally
with try-with-resources.
Posted by GitBox <gi...@apache.org>.
ctubbsii merged pull request #1546: Replace try/finally with try-with-resources.
URL: https://github.com/apache/accumulo/pull/1546
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [accumulo] ctubbsii commented on a change in pull request #1546:
Replace try/finally with try-with-resources.
Posted by GitBox <gi...@apache.org>.
ctubbsii commented on a change in pull request #1546: Replace try/finally with try-with-resources.
URL: https://github.com/apache/accumulo/pull/1546#discussion_r388351313
##########
File path: core/src/main/java/org/apache/accumulo/core/file/rfile/bcfile/PrintInfo.java
##########
@@ -39,10 +39,8 @@
public static void printMetaBlockInfo(SiteConfiguration siteConfig, Configuration conf,
FileSystem fs, Path path) throws IOException {
FSDataInputStream fsin = fs.open(path);
- BCFile.Reader bcfr = null;
- try {
- bcfr = new BCFile.Reader(fsin, fs.getFileStatus(path).getLen(), conf,
- CryptoServiceFactory.newInstance(siteConfig, ClassloaderType.ACCUMULO));
+ try (BCFile.Reader bcfr = new BCFile.Reader(fsin, fs.getFileStatus(path).getLen(), conf,
Review comment:
That's fine. We can merge this without those suggestions. They aren't necessary.
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [accumulo] ctubbsii commented on a change in pull request #1546:
Replace try/finally with try-with-resources.
Posted by GitBox <gi...@apache.org>.
ctubbsii commented on a change in pull request #1546: Replace try/finally with try-with-resources.
URL: https://github.com/apache/accumulo/pull/1546#discussion_r387943345
##########
File path: core/src/main/java/org/apache/accumulo/core/file/rfile/bcfile/PrintInfo.java
##########
@@ -39,10 +39,8 @@
public static void printMetaBlockInfo(SiteConfiguration siteConfig, Configuration conf,
FileSystem fs, Path path) throws IOException {
FSDataInputStream fsin = fs.open(path);
- BCFile.Reader bcfr = null;
- try {
- bcfr = new BCFile.Reader(fsin, fs.getFileStatus(path).getLen(), conf,
- CryptoServiceFactory.newInstance(siteConfig, ClassloaderType.ACCUMULO));
+ try (BCFile.Reader bcfr = new BCFile.Reader(fsin, fs.getFileStatus(path).getLen(), conf,
Review comment:
```suggestion
try (var bcfr = new BCFile.Reader(fsin, fs.getFileStatus(path).getLen(), conf,
```
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [accumulo] ctubbsii commented on a change in pull request #1546:
Replace try/finally with try-with-resources.
Posted by GitBox <gi...@apache.org>.
ctubbsii commented on a change in pull request #1546: Replace try/finally with try-with-resources.
URL: https://github.com/apache/accumulo/pull/1546#discussion_r387942830
##########
File path: core/src/main/java/org/apache/accumulo/core/clientImpl/InstanceOperationsImpl.java
##########
@@ -188,17 +187,12 @@ public boolean testClassLoad(final String className, final String asTypeName)
@Override
public void ping(String tserver) throws AccumuloException {
- TTransport transport = null;
- try {
- transport = createTransport(AddressUtil.parseAddress(tserver, false), context);
- var client = createClient(new TabletClientService.Client.Factory(), transport);
+ try (
+ TTransport transport = createTransport(AddressUtil.parseAddress(tserver, false), context)) {
Review comment:
```suggestion
try (var transport = createTransport(AddressUtil.parseAddress(tserver, false), context)) {
```
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [accumulo] jmark99 commented on a change in pull request #1546:
Replace try/finally with try-with-resources.
Posted by GitBox <gi...@apache.org>.
jmark99 commented on a change in pull request #1546: Replace try/finally with try-with-resources.
URL: https://github.com/apache/accumulo/pull/1546#discussion_r388272607
##########
File path: core/src/main/java/org/apache/accumulo/core/file/rfile/bcfile/PrintInfo.java
##########
@@ -39,10 +39,8 @@
public static void printMetaBlockInfo(SiteConfiguration siteConfig, Configuration conf,
FileSystem fs, Path path) throws IOException {
FSDataInputStream fsin = fs.open(path);
- BCFile.Reader bcfr = null;
- try {
- bcfr = new BCFile.Reader(fsin, fs.getFileStatus(path).getLen(), conf,
- CryptoServiceFactory.newInstance(siteConfig, ClassloaderType.ACCUMULO));
+ try (BCFile.Reader bcfr = new BCFile.Reader(fsin, fs.getFileStatus(path).getLen(), conf,
Review comment:
@ctubbsii thanks for the info. I'm not that familiar with the use of var at the moment so I will read up on it a bit and then see if I can apply it where appropriate. You can take another look once I push another update.
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [accumulo] ctubbsii commented on issue #1546: Replace try/finally
with try-with-resources.
Posted by GitBox <gi...@apache.org>.
ctubbsii commented on issue #1546: Replace try/finally with try-with-resources.
URL: https://github.com/apache/accumulo/pull/1546#issuecomment-595317766
@jmark99 I went ahead and merged what you had, since it was fine. Any `var` improvements can be done as follow-on... if you even want to (it's not really necessary).
----------------------------------------------------------------
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
With regards,
Apache Git Services