Skip to content

Conversation

@fifco
Copy link

@fifco fifco commented Apr 6, 2015

Useful for hosting where From email headers in php mail() are
restricted to concrete ones. Can be configured through neon.

  • feature
  • documentation - needed
  • BC break - no

Filip Čonka added 2 commits April 6, 2015 19:15
Useful for hostings where From email headers in php mail() are
restricted to concrete ones. Can be configured through neon.
Managed to fix whitespaces :)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't like this new property and I see no reason for the debugger to be concerned about what the Logger is doing.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

But it already is by managing logDirectory and email.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's for BC, I don't see any reason to introduce more properties like this on the Debugger.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

But it will not be able to configure it from neon in 'debugger' section, will it?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why not?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok, it does not need to be passed to Logger class but it must stay as property in Debugger class. (I am going to push new commit about this in few sec). Because when I want to configure fromEmail in neon from 'debugger' section it must be declared as static property in Debugger class, or am I wrong?

@dg
Copy link
Member

dg commented Apr 6, 2015

Logger::$fromEmail is OK, but please do not add new argument to methods.

@dg dg closed this in 37206e0 May 3, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants