Who thinks that this is a pressing issue for users? (There are lots of those.) The only pressing issue I see discussed here is that some loggers are labeled incorrectly causing debugging problems. Let's just fix those and focus on what matters to users (e.g. Critical and Major bugs).
As an aside, big changes like this are disruptive to the many other outstanding patches, are hard to maintain, rebase, merge, etc. There should be a huge benefit to this type of wholesale change (to balance the cost). None of the arguments Daniel made achieve that criteria for me. ...just my .02 -- Jacques Nadeau CTO and Co-Founder, Dremio On Mon, Jul 27, 2015 at 2:52 PM, Daniel Barclay <[email protected]> wrote: > > I can't seem to find any guidance for this particular issue. > > Yeah; I wouldn't expect anything specific to our peculiar pattern, > other than a general endorsement of usually using imports and usually > not using fully-qualified names. > > Is there any reason not to go with imports and non-qualified names > (with the parts I mentioned below to retain the easy commenting/ > uncommenting to avoid unused imports and loggers)? > > > If there's concern about having loggers in both styles for a long > interim period: I think I could convert the declarations pretty > rapidly (using some Emacs regular-expression bulk replacement), as > long as I can get someone to approve and merge in the changes as > quickly as we want the inconsistency to go away. > > Daniel > > > Chris Westin wrote: > >> I tried to follow up with Hakim's suggestion of consulting the checkstyle >> rules I expect to use (I've suggested before that we should start with >> Google's rules as a basis, and make a few tweaks), but unfortunately, on >> that day last week, SourceForge was down (that's where the rules are >> hosted). It's finally back, so here they are >> http://checkstyle.sourceforge.net/google_style.html . I can't seem to >> find >> any guidance for this particular issue. >> >> On Wed, Jul 22, 2015 at 10:51 AM, Daniel Barclay <[email protected]> >> wrote: >> >> >>> Chris Westin wrote: >>> >>> For the special case of the logger, I kind of like it this way, because >>>> I >>>> can turn it off just by commenting out a single line (to get rid of >>>> unreferenced variable warnings),or add it by pasting in or uncommenting >>>> a >>>> single line. In either case I don't have to worry about removing or >>>> adding >>>> the import line separately, which can be quite far away if there are a >>>> lot >>>> of imports. >>>> >>>> >>> Why not use the modern Java feature intended for cases like this: have >>> a @SuppressWarnings("unused") annotation on the logger member >>> declaration if the declaration has been added but the member isn't used >>> yet? >>> >>> Then: >>> - We can still avoid unused-variable warnings for logger members that >>> have already been declared before there are any uses. >>> - We no longer have to move up top to adjust an already-existing logger >>> declaration when adding a logger use down in the code. >>> (Yes, we should remove (or comment out) the annotation if the change >>> isn't temporary, but we don't have to do that immediately just to >>> continue compiling.) >>> - We can now use code completion when adding the first logger call to a >>> previously unused logger, since the declaration is real (not just a >>> comment). >>> - We can still comment out/uncomment only a single line (the annotation) >>> to switch between the no-logger-uses and some-logger-use cases. (That >>> is, if you don't want to have to re-add the suppression annotation if >>> the last use of a logger is removed later, you can comment out, rather >>> than delete, the annotation when the first logger use it added. >>> - We no longer have to either go adjust imports or use qualified names >>> to avoid having to adjust imports. >>> - We stop having unnecessarily qualified names in the code. >>> - and, finally ...: >>> - We stop having those names' extra visual clutter and length, which >>> makes it harder to notice when the class literal ends up wrong. >>> (Note the mention of "pasting" above.) >>> >>> Daniel >>> >>> >>> >>> >>> >>> >>> On Tue, Jul 21, 2015 at 6:12 PM, Daniel Barclay <[email protected]> >>>> wrote: >>>> >>>> For logger member declarations, can we drop the current pattern of >>>> using >>>> >>>>> qualified names (like this: >>>>> >>>>> private static final org.slf4j.Logger logger = >>>>> org.slf4j.LoggerFactory.getLogger(StoragePluginRegistry.class); >>>>> >>>>> ) and allow using imports and non-qualified names (as we do for almost >>>>> everything else)? >>>>> >>>>> >>>>> Using qualified names adds a lot of visual noise, and pushes the class >>>>> literal farther to the right, making it easier to fail to notice that >>>>> it doesn't match the containing class. >>>>> >>>>> Thanks, >>>>> Daniel >>>>> -- >>>>> Daniel Barclay >>>>> MapR Technologies >>>>> >>>>> >>>>> >>>> >>> -- >>> Daniel Barclay >>> MapR Technologies >>> >>> >> > > -- > Daniel Barclay > MapR Technologies >
