milleruntime commented on pull request #1774: URL: https://github.com/apache/accumulo/pull/1774#issuecomment-726175596
> Aside from a few String-based concerns, my main concern with these changes is that "location" means too many different things. In some contexts, it means "a specific tserver instance", but in other contexts the same word "location" means "host address and port". It gets more complicated when we pass a location containing a location, or assign a location variable named loc to some tserver's location's location. > > In some places, it's very clear, we just mean "host address and port (as a String)". It might be nice to have that kind of clarity everywhere, instead of trying to figure out which "location" we mean every time we see it. This might just be a matter of choosing better variable and method names where these types are used, to accompany your effort to simplify the types/classes. I agree. Your concerns with location terminology is prevalent everywhere across our code. These changes were just a first pass, moving the master classes to core and combining the inner classes. I can improve the methods in TServerInstance as well in this PR but I didn't want to make too many changes at once. ---------------------------------------------------------------- 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]
