Enable debug without constants (issue #62) #63

Closed
wants to merge 2 commits into
from

Conversation

Projects
None yet
5 participants

Added support for callers to enable and disable channel debug output
without setting a constant.

#62
Added support for callers to enable and disable channel debug output
without setting a constant.
Collaborator

videlalvaro commented Jan 17, 2013

Would you mind updating the README as well. So next to where it mentions the AMQP_DEBUG constant, also document this behaviors

How about adding this as a constructor argument?

Collaborator

videlalvaro commented Jan 17, 2013

@schmittjoh I thought about it… but there are way too many already, but it could be an option to consider

Maybe add a builder class for the connection?

ConnectionBuilder::create()
   ->setHost()
   // ...
   ->enableDebug()
   ->build();

I guess it depends on whether or not debug should be global, or on a channel-by-channel basis.

Contributor

fprochazka commented Jun 21, 2014

The "constants configuration" is actually pretty awfull. I really like this change, will you please rebase it so it can be merged?

Collaborator

videlalvaro commented Jun 21, 2014

Note that due to performance reasons we need this change to happen before any connection is attempted, which probably means changing the API

Contributor

fprochazka commented Jun 21, 2014

@videlalvaro even when the act of connecting is lazy?

Collaborator

videlalvaro commented Jun 21, 2014

If the connection is lazy, then that's fine

Contributor

fprochazka commented Jun 21, 2014

I'm debating whether whether I should send a pull in which I'd make this always lazy, because there is really no reason not to be always lazy, or implement this enableDebugmethod in the upper application layer by extending the connection class. Any thoughts?

Collaborator

videlalvaro commented Jun 21, 2014

As always keep in mind other people code, we don't want to break things :)

Contributor

fprochazka commented Jun 21, 2014

I know that, but I hoped you'd tell me if you know about some possible issues when you not connect lazily. So I could fix them, or drop the case if not possible.

Contributor

postalservice14 commented Oct 6, 2015

I think this PR can be closed. Since the $debug variable is no longer a boolean that can be set.

Collaborator

videlalvaro commented Oct 6, 2015

Yup. I think now the right thing to do would be to introduce a better constructor API for the AMQPConnection* classes that's able to accept a DebugHelper class instance.

@videlalvaro videlalvaro closed this Oct 6, 2015

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment