Manno15 commented on pull request #1972: URL: https://github.com/apache/accumulo/pull/1972#issuecomment-800990172
I like your additions. Some comments below, in order of the commit points. - The renaming was going to happen in #1967 which is why I didn't do it here. - I did have something akin to this in my original commit but wasn't quite sure if it was better or not. So I removed it in the next commit. I did it this way so I can drop the second one if I changed my mind about it. I do personally like having the path function better though. - I am indifferent to the `var` and method name change. I agree the shorter name is probably better, especially with a comment on the method now to explain its intent. - Good change, I thought the fields were already final so that was my mistake. - Make sense if it wasn't used. Good change. Everything else looks good. ---------------------------------------------------------------- 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: [email protected]
