Thanks for the ping here Stack. Posted review on the jira. Summary is, we need at least: 1) basic verification 2) fatal markers 3) clear picture on properties file: Is old one compatible? if not, we need new ones and document what's breaking. New ones and documentation can be done in followup, but we need clear picture now. 4) Followup jira for hbase-backup/http or a reason why they can't be done.
Reverting this patch would be hell, wouldn't want to do it because we missed basic checks. -- Appy On Mon, Dec 18, 2017 at 3:53 PM, Apekshit Sharma <a...@cloudera.com> wrote: > Oh, just 60 pages of review :D > > -- Appy > > On Mon, Dec 18, 2017 at 3:48 PM, Stack <st...@duboce.net> wrote: > >> On Wed, Dec 13, 2017 at 10:09 AM, Stack <st...@duboce.net> wrote: >> >> > On Mon, Dec 11, 2017 at 12:49 PM, Apekshit Sharma <a...@cloudera.com> >> > wrote: >> > >> >> Seems like good idea: >> >> - remove long dead dependency >> >> - a bit cleaner code >> >> - hadoop also moved to slf4j >> >> >> >> Quickly looking at codebase to get idea of amount of work required, >> here >> >> are some numbers: >> >> - LOG.debug : ~1800 >> >> - LOG.trace : ~500 >> >> - LOG.info: ~3000 >> >> >> >> Looking at this patch ( >> >> https://issues.apache.org/jira/secure/attachment/12901002/ >> >> HBASE-19449.1.patch), >> >> seemed like tedious and repetitive task, was wondering if someone has >> >> automated it already. >> >> Looks like this can help reduce a huge part of grunt work: >> >> https://www.slf4j.org/migrator.html. >> >> >> >> But before progressing, as a basic validation, can we see: >> >> - an example of old vs new log lines (that there is no diff, or we are >> >> comfortable with what's there) >> >> - an example of changes in properties file >> >> >> >> Maybe starting with hbase-examples module for quick POC. >> >> >> >> >> > I like your suggestion Appy, >> > S >> > >> > >> Balazs has a patch up on HBASE-10092 making the move. Intend to commit it >> in next day or so unless objection. >> S >> >> >> >> > >> > >> >> -- Appy >> >> >> > >> > >> > > > > -- > > -- Appy > -- -- Appy
