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/09/08 17:50:15 UTC

[GitHub] [accumulo] ctubbsii opened a new pull request #1702: Clean up KeyExtent

ctubbsii opened a new pull request #1702:
URL: https://github.com/apache/accumulo/pull/1702


   * Replace confusing overloaded constructors with self-descriptive
     static factory methods
   * Move metadata serialization into Metadata schema class
   * Remove unnecessary WritableComparable interface
   * Improve javadocs
   * Replace private equals method with Objects.equals
   * Remove methods only used in tests
   * Use TableId instead of String and fix variable name (tableId
     instead of incorrect tablename)
   * Shorten accessor method names
   * Make KeyExtent closer to immutable (it's not truly immutable,
     because Text objects that represent endRow and prevEndRow are
     mutable)


----------------------------------------------------------------
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

[GitHub] [accumulo] ctubbsii commented on pull request #1702: Clean up KeyExtent

Posted by GitBox <gi...@apache.org>.
ctubbsii commented on pull request #1702:
URL: https://github.com/apache/accumulo/pull/1702#issuecomment-689232327


   My run of the full ITs passed.


----------------------------------------------------------------
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

[GitHub] [accumulo] ctubbsii commented on pull request #1702: Clean up KeyExtent

Posted by GitBox <gi...@apache.org>.
ctubbsii commented on pull request #1702:
URL: https://github.com/apache/accumulo/pull/1702#issuecomment-689049275


   I know this is a big PR, but most of this was done with very simple automated refactorings in the IDE... it just looks like a lot more was done, because KeyExtent is used in a lot of files.
   
   I think these changes are worth it, because they make dealing with KeyExtent a bit less error prone, especially with the various `Text` parameters that had different semantics in the overloaded constructors. Giving names to those overloaded constructors, and eliminating the setters makes this object safer to use.
   
   I'm currently running the full ITs on this before I'd consider merging.


----------------------------------------------------------------
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

[GitHub] [accumulo] ctubbsii merged pull request #1702: Clean up KeyExtent

Posted by GitBox <gi...@apache.org>.
ctubbsii merged pull request #1702:
URL: https://github.com/apache/accumulo/pull/1702


   


----------------------------------------------------------------
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