[GitHub] [hbase] saintstack commented on issue #1062: HBASE-23705 Add CellComparator to HFileContext

2020-01-22 Thread GitBox
saintstack commented on issue #1062: HBASE-23705 Add CellComparator to 
HFileContext
URL: https://github.com/apache/hbase/pull/1062#issuecomment-577493606
 
 
   Unit test is known issue (HBASE-23722). The checkstyle is the 
method-too-large. 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:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] [hbase] saintstack commented on issue #1062: HBASE-23705 Add CellComparator to HFileContext

2020-01-22 Thread GitBox
saintstack commented on issue #1062: HBASE-23705 Add CellComparator to 
HFileContext
URL: https://github.com/apache/hbase/pull/1062#issuecomment-577259480
 
 
   Checkstyle failure is for the method that is too long (expected). The failed 
test passes locally. Let me rerun to make sure.


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:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] [hbase] saintstack commented on issue #1062: HBASE-23705 Add CellComparator to HFileContext

2020-01-21 Thread GitBox
saintstack commented on issue #1062: HBASE-23705 Add CellComparator to 
HFileContext
URL: https://github.com/apache/hbase/pull/1062#issuecomment-577042490
 
 
   The checkstyle complaints are two. One is an old method that is > 150 limit. 
It is 242 currently (after I tried cutting it down). Leaving it. Not related. 
Other is an import order I finally figured.
   
   On unit tests, I tried them locally and they fail; a checkstyle change 
suggesting class be final broke mockito. Put up new patch.
   
   Thanks for +1 @anoopsjohn 


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:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] [hbase] saintstack commented on issue #1062: HBASE-23705 Add CellComparator to HFileContext

2020-01-21 Thread GitBox
saintstack commented on issue #1062: HBASE-23705 Add CellComparator to 
HFileContext
URL: https://github.com/apache/hbase/pull/1062#issuecomment-576882122
 
 
   Address review comments. Ran through files I touched with checkstyle.
   
   Looking for a +1 please. Thanks.


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:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] [hbase] saintstack commented on issue #1062: HBASE-23705 Add CellComparator to HFileContext

2020-01-20 Thread GitBox
saintstack commented on issue #1062: HBASE-23705 Add CellComparator to 
HFileContext
URL: https://github.com/apache/hbase/pull/1062#issuecomment-576339559
 
 
   @ramkrish86 and @anoopsjohn  Thanks for the reviews. Helps. You object to 
the reshuffle suggesting that the encoder keep its comparator manipulations 
local. I tried that initially but when it meant trying to figure which 
cellcomparator when, and then casting the comparator and trying to figure how 
to do meta cell comparator when no current support, and then the thought that 
any encoder that needs to change dependent on the table they are running 
against would have to do the same thing, it made me take pause and do this more 
fundamental change.
   
   Let me know if my argument does not convince and I'll just hack up something 
that works locally for this one encoder. Thanks.


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:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] [hbase] saintstack commented on issue #1062: HBASE-23705 Add CellComparator to HFileContext

2020-01-19 Thread GitBox
saintstack commented on issue #1062: HBASE-23705 Add CellComparator to 
HFileContext
URL: https://github.com/apache/hbase/pull/1062#issuecomment-576048953
 
 
   Thank you for the review @HorizonNet  Will put up a patch in a while that 
implements your nits.


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:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] [hbase] saintstack commented on issue #1062: HBASE-23705 Add CellComparator to HFileContext

2020-01-18 Thread GitBox
saintstack commented on issue #1062: HBASE-23705 Add CellComparator to 
HFileContext
URL: https://github.com/apache/hbase/pull/1062#issuecomment-575965360
 
 
   Review if anyone has a chance. Thanks.


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:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] [hbase] saintstack commented on issue #1062: HBASE-23705 Add CellComparator to HFileContext

2020-01-18 Thread GitBox
saintstack commented on issue #1062: HBASE-23705 Add CellComparator to 
HFileContext
URL: https://github.com/apache/hbase/pull/1062#issuecomment-575909543
 
 
   Undid mistaken setting of encoding and blooms on meta schema. Thats for 
HBASE-21065.


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:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] [hbase] saintstack commented on issue #1062: HBASE-23705 Add CellComparator to HFileContext

2020-01-18 Thread GitBox
saintstack commented on issue #1062: HBASE-23705 Add CellComparator to 
HFileContext
URL: https://github.com/apache/hbase/pull/1062#issuecomment-575907442
 
 
   Fix checkstyle


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:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] [hbase] saintstack commented on issue #1062: HBASE-23705 Add CellComparator to HFileContext

2020-01-17 Thread GitBox
saintstack commented on issue #1062: HBASE-23705 Add CellComparator to 
HFileContext
URL: https://github.com/apache/hbase/pull/1062#issuecomment-575869743
 
 
   Fix complaints and UTs.


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:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] [hbase] saintstack commented on issue #1062: HBASE-23705 Add CellComparator to HFileContext

2020-01-17 Thread GitBox
saintstack commented on issue #1062: HBASE-23705 Add CellComparator to 
HFileContext
URL: https://github.com/apache/hbase/pull/1062#issuecomment-575812631
 
 
   Patch looks big but core changes are small just having cellcomparator in 
context only and not all over the place.


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:
us...@infra.apache.org


With regards,
Apache Git Services