Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Updating recommended email settings for monolog #6587

Closed
wants to merge 3 commits into from

Conversation

jorgelbg
Copy link
Contributor

Q A
Doc fix? yes
New docs? no
Applies to >=2.3
Fixed tickets #6467

'buffered' => array(
'type' => 'buffer',
'handler' => 'swift',
'duplicated' => array(
Copy link
Member

Choose a reason for hiding this comment

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

Should be deduplicated here?

@Txumari
Copy link

Txumari commented May 21, 2016

I'm new here, but I think should be change the buffered explanation with the new handler deduplicated (The deduplicated handler simply keeps...)

@jorgelbg
Copy link
Contributor Author

Fixed the typos, and amended the commit. Thanks @javiereguiluz @xabbuh

@xabbuh
Copy link
Member

xabbuh commented May 21, 2016

👍

@javiereguiluz
Copy link
Member

👍

(by the way, I really dislike the "deduplication" word ... but that's something we cannot change)

@wouterj
Copy link
Member

wouterj commented May 21, 2016

I agree with @Txumari here, after this code block we talk about the buffered handler a couple times. We should change it to deduplicated handler (and also change the description).

@jorgelbg
Copy link
Contributor Author

jorgelbg commented May 21, 2016

Should we mantain some information about the buffer handler? its still a valid handler according to https://github.com/Seldaek/monolog/blob/master/doc/02-handlers-formatters-processors.md#wrappers--special-handlers but honestly the deduplication handler does also the buffering + deduplication of messages. The final example use the group and buffer.

…emoving some mentions of the buffer handler

Fixing some syntax issues
@jorgelbg
Copy link
Contributor Author

I've updated the explanation and fixed the mentions of the buffered handler, also changed the last example in the doc, that talked about combining the handlers to store the messages on the server and also send the logs by email.

unique over a given period of time (60 seconds by default). If the records are
duplicates they are simply discarded. Adding this handler reduces the amount of
notifications to a manageable level, specially in critical failure scenarios.
The messages are then passed to the ``swift`` handler. This is the handler that
Copy link
Member

Choose a reason for hiding this comment

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

I would add a blank line before this sentence to have it rendered as its own paragraph to improve readability.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done!

@wouterj
Copy link
Member

wouterj commented Jun 11, 2016

👍 I like it

Status: Reviewed

@xabbuh
Copy link
Member

xabbuh commented Jun 20, 2016

Thank you @jorgelbg.

xabbuh added a commit that referenced this pull request Jun 20, 2016
This PR was submitted for the 2.3 branch but it was merged into the 2.7 branch instead (closes #6587).

Discussion
----------

Updating recommended email settings for monolog

| Q             | A
| ------------- | ---
| Doc fix?      | yes
| New docs?     | no
| Applies to    | >=2.3
| Fixed tickets | #6467

Commits
-------

133d36b Updating recommended email settings for monolog
@xabbuh xabbuh closed this Jun 20, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants