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

Enhancement 1 #114

Open
wants to merge 13 commits into
base: master
Choose a base branch
from
Open

Enhancement 1 #114

wants to merge 13 commits into from

Conversation

skorhone
Copy link

@skorhone skorhone commented Sep 5, 2016

Multiple fixes / enhancements

@t0xa
Copy link
Owner

t0xa commented Sep 6, 2016

Hi @skorhone,

Wow, excellent work! It will take some time for me to review and merge changes.
How urgently you need new version?

Anton

@skorhone
Copy link
Author

skorhone commented Sep 8, 2016

Hi Anton,

it would be great, if changes could be merged. There's no hurry, as we're
currently building our production versions from our fork

You might be more interested in master branch (trunk) It contains quite a
few additions.

The most notable feature is support for sending messages asynchronously
using dedicated dispatcher thread. I decided to add this feature, because
losing messages occasionally is usually preferred over having
application/application server halt.

Second useful addition is GelfHTTPSender. This is obviously restful client
for graylog

Third somewhat notable change is the way JSON is built. I added a simple
(well tested) JSON string encoder to remove dependency from 3rd party json
library. JSON library may and propably will cause conflict on EE7
application servers. This change adds some limitations to custom fields
(fields may only be primitives or strings), but I personally think that
this limitation is reasonable.

And finally, I've tried to improve unit test coverage on parts where it
matters the most. It's still far from perfect :-)

If you have any comments regarding coding style / unit test coverage,
please let me know.

Best regards,
Sami Korhonen

On Tue, Sep 6, 2016 at 11:20 PM, Anton Yakimov notifications@github.com
wrote:

Hi @skorhone https://github.com/skorhone,

Wow, excellent work! It will take some time for me to review and merge
changes.
How urgently you new version?

Anton


You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
#114 (comment), or mute
the thread
https://github.com/notifications/unsubscribe-auth/AEl6yOT6bCGrb_DbnwOU1_zUyQh-n6Vkks5qncr1gaJpZM4J068O
.

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.

2 participants