search

Medium / matador

an MVC framework for Node
http://medium.github.io/matador
Other
604 stars 49 forks source link

Request logger #128

Closed vinibaggio closed 11 years ago

vinibaggio commented 11 years ago

Hello @dpup,

Please review the following commits I made in branch 'vinny-request-logger'.

I created a RequestMessage, which is basically a string builder with request info. I also added a default development environment request logger to console.info, but in different environments someone can just use a logger they want and a Request Message they want.

Request message is the class responsible for building the message. You can plug new "request events" such as database calls. This mechanism may need some further review, but it is enough for my own scenario.

2c136ce95bb4a6ac2228a90b5be27cbf6a0aafb1 (2013-07-18 17:54:31 -0700) Creating a default request logger

R=@dpup

dpup commented 11 years ago

I wonder if the event registration is a bit overkill. Also, might be worth thinking of a different terminology, "event" is already so overloaded.

-- Dan

On Thu, Jul 18, 2013 at 6:01 PM, Vinicius Baggio Fuentes < notifications@github.com> wrote:

Hello @dpup https://github.com/dpup,

Please review the following commits I made in branch 'vinny-request-logger'.

I created a RequestMessage, which is basically a string builder with request info. I also added a default development environment request logger to console.info, but in different environments someone can just use a logger they want and a Request Message they want.

Request message is the class responsible for building the message. You can plug new "request events" such as database calls. This mechanism may need some further review, but it is enough for my own scenario.

2c136cehttps://github.com/Obvious/matador/commit/2c136ce95bb4a6ac2228a90b5be27cbf6a0aafb1(2013-07-18 17:54:31 -0700) Creating a default request logger

R=@dpup https://github.com/dpup

You can merge this Pull Request by running

git pull https://github.com/Obvious/matador vinny-request-logger

Or view, comment on, or merge it at:

https://github.com/Obvious/matador/pull/128 Commit Summary

  • Creating a default request logger

File Changes

  • M package.jsonhttps://github.com/Obvious/matador/pull/128/files#diff-0(3)
  • A src/RequestMessage.jshttps://github.com/Obvious/matador/pull/128/files#diff-1(140)
  • M src/matador.jshttps://github.com/Obvious/matador/pull/128/files#diff-2(24)
  • A tests/functional/app/config/routes.jshttps://github.com/Obvious/matador/pull/128/files#diff-3(5)
  • A tests/functional/app/controllers/HomeController.jshttps://github.com/Obvious/matador/pull/128/files#diff-4(16)
  • A tests/functional/logger_test.jshttps://github.com/Obvious/matador/pull/128/files#diff-5(34)
  • A tests/support/functional.jshttps://github.com/Obvious/matador/pull/128/files#diff-6(101)

Patch Links:

vinibaggio commented 11 years ago

I agree with the terminology, but I don't think it's overkill. The good thing about it is that it's flexible so in your apps you can add more events, such as database calls, cache hit/miss, etc, and the impl is quite simple.