Github user dsbos commented on the pull request:
https://github.com/apache/drill/pull/116#issuecomment-137653774
> Can you start by reviewing my branch? Let's get it right before we try
rebasing.
I'm confused by that part of your comment. I have been reviewing your
branch, and I think I've reviewed the latest (with-test/without-ProGuard)
version of it (hence my earlier review comments, and your replies to them).
Have I missed a message or am I looking at the wrong thing? (I have been
and am looking at this [Pull Request
#116](https://github.com/apache/drill/pull/116).)
Also, your mention of rebasing sounds like a reference to a preceding
request to rebase, but I don't see or recall any mention of rebasing your patch.
Looking again, I see that I just skimmed over the class loader that's just
for the test. Is it that you wanted more review of that class?
Oh--it is that I hadn't indicated that I was done reviewing it because I
didn't give a +1 yet? I hadn't done that before because I was waiting for
confirmation on my SLF4J question. You have answered that now (below), so the
patch seems "plus-oneable" unless the resolution of whatever the confusion is
here reveals a problem with the patch.
> With regards to slf4j and logging: ... We support any slf4j logging tool
but we don't package any. ...
> If we include a logging framework, then a user can't use their own for
centralized logging...
Roger. Yeah, I thought it might be that.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at [email protected] or file a JIRA ticket
with INFRA.
---
