search

mendicant-original / newman

Newman: a microframework for building email-based applications.
116 stars 11 forks source link

app logging - 2nd try #21

Closed ericgj closed 12 years ago

ericgj commented 12 years ago

see issue #16

practicingruby commented 12 years ago

Left some comments on the commit. Generally speaking I'd like to see the RequestLogger and ResponseLogger be ordinary class definitions which wrap a Logger instance and implement a call method. These can live in _lib/newman/requestlogger.rb and _lib/newman/responselogger.rb. Your change to make the system run multiple apps again looks fine to me.

ericgj commented 12 years ago

Thanks, great suggestions! I must admit to being a bit brain-dead last night. I re-did it here, mostly along the lines you suggested above, except:

  1. There is just one Logger class that accepts flag params log_request, log_response -- in addition to the logger. That way you could log both request and response at the same point if you wanted.
  2. The settings are logged as well as the request and/or response, at debug level
  3. I did not yet move the code to lib/newman/logger.rb, it's still under examples/logging.rb
practicingruby commented 12 years ago

Hi Eric. You don't need to link to a commit as long as you reference the pull request number in the commit, it will add it to this pull request.

I left some comments on that commit though. I think that you've given me some solid ideas on how to solve this problem, and that it'd be best for me just to wrap up this patch in the interest of time.

Thanks. -greg

practicingruby commented 12 years ago

I've pushed some commits which I think will work for now. Thanks for your help on this Eric.