Project

General

Profile

Actions

Feature #5096

closed

offer Logger-compatibility for syslog ext

Added by normalperson (Eric Wong) over 12 years ago. Updated almost 12 years ago.

Status:
Closed
Target version:
-
[ruby-core:38503]

Description

There's http://rubygems.org/gems/SyslogLogger, but I would like to have
ths compatibility by default. Having to remember to install the
oddly-capitalized SyslogLogger gem makes things harder than it should
be.

Importing the SyslogLogger gem would solve this issue.


Files

sysloglogger.patch (19.6 KB) sysloglogger.patch tenderlovemaking (Aaron Patterson), 03/09/2012 07:24 AM
syslog.logger.patch (20.2 KB) syslog.logger.patch rails-free patch drbrain (Eric Hodel), 05/16/2012 09:59 AM
knu-syslog_logger.patch.txt (5.67 KB) knu-syslog_logger.patch.txt knu (Akinori MUSHA), 05/17/2012 09:59 PM

Updated by normalperson (Eric Wong) over 12 years ago

Bah, I submitted the bug before I finished typing :<

Title should be: "offer Logger-compatibility for syslog ext"
Priority should be: Low

There's http://rubygems.org/gems/SyslogLogger, but I would like to have
ths compatibility by default. Having to remember to install the
oddly-capitalized SyslogLogger gem makes things harder than it should
be.

Importing the SyslogLogger gem would solve this issue.

Updated by nahi (Hiroshi Nakamura) over 12 years ago

  • Subject changed from offer Logger-compatibility for ext to offer Logger-compatibility for syslog ext
  • Priority changed from Normal to 3

Updated by drbrain (Eric Hodel) over 12 years ago

I would be happy to import my SyslogLogger gem, but I need someone to decide if it is ok first.

Updated by naruse (Yui NARUSE) over 12 years ago

Where is a patch or the repository?

Anyway such enhancement should be introduced as a patch to trunk.
We are hard to understand the actual change of this feature.

Moreover adding a new library or replacing a library is hard to accepted.
Enhance a existed library is much easier then it.

Updated by drbrain (Eric Hodel) about 12 years ago

  • Assignee set to drbrain (Eric Hodel)

The repository is here: https://github.com/seattlerb/sysloglogger

I will attach a patch to this issue soon.

Updated by tenderlovemaking (Aaron Patterson) about 12 years ago

I've attached a patch. It imports sysloglogger as Syslog::Logger. Syslog::Logger is a logger that logs to Syslog, but has the same API as Logger.

Updated by drbrain (Eric Hodel) about 12 years ago

I think the implementation is fine, but the Syslog::Logger constant should be updated since the name was changed.

Also, should the rails references be removed or changed?

Updated by drbrain (Eric Hodel) about 12 years ago

Ugh, I meant "Syslog::Logger comment"

Actions #9

Updated by shyouhei (Shyouhei Urabe) about 12 years ago

  • Status changed from Open to Assigned

Updated by drbrain (Eric Hodel) almost 12 years ago

This updated patch removes references to rails and the silence method (as it is not thread-safe).

Updated by knu (Akinori MUSHA) almost 12 years ago

  • Assignee changed from drbrain (Eric Hodel) to knu (Akinori MUSHA)

I'll look into this shortly.

I think I'm going to put this under ext/syslog/lib/syslog/ so the whole syslog library can be made a gem in the future.

Updated by drbrain (Eric Hodel) almost 12 years ago

The second rails-free patch places it under ext/syslog/lib/syslog and introduces test/syslog/.

I did not move test/test_syslog.rb to test/syslog/test_syslog.rb though, I was going to submit this as a separate issue if this was was accepted.

Updated by knu (Akinori MUSHA) almost 12 years ago

Oh, of course. I didn't carefully read the latest patch.

I've revised the implementation and the tests as attached.

Here's the list of what I changed:

  • Reduced map constants. You shouldn't need this many.

  • Changed Syslog::Logger::SYSLOG to Syslog::Logger.syslog using a class variable as storage. I didn't like the way the constant was used.

  • Changed to call syslog methods with ('%s', message) instead of manually replacing %'s.

  • Changed a regexp so it exactly matches (ANSI & XTerm) color escape sequences.

Please merge it if it's OK, and you have my approval.

Updated by drbrain (Eric Hodel) almost 12 years ago

I think you forgot to attach the patch.

Your changes sound good, thank you for the review and update on my very (5 year) old code.

Updated by knu (Akinori MUSHA) almost 12 years ago

D'oh, here it is.

Actions #17

Updated by drbrain (Eric Hodel) almost 12 years ago

  • Status changed from Assigned to Closed
  • % Done changed from 0 to 100

This issue was solved with changeset r35682.
Eric, thank you for reporting this issue.
Your contribution to Ruby is greatly appreciated.
May Ruby be with you.


  • ext/syslog/lib/syslog/logger.rb: Added Syslog::Logger which was
    ported from the SyslogLogger gem. [ruby-trunk - Feature #5096]
  • NEWS: ditto.
  • test/syslog/test_syslog_logger.rb: ditto.
Actions

Also available in: Atom PDF

Like0
Like0Like0Like0Like0Like0Like0Like0Like0Like0Like0Like0Like0Like0Like0Like0Like0Like0