Added the option to give the custom name for the logger method.

lulalala / multi_logger

Create multiple loggers in Rails

MIT License

59 stars 21 forks source link

Added the option to give the custom name for the logger method. #7

Closed karuppasamy closed 10 years ago

karuppasamy commented 10 years ago

I have added the new option to give a custom name to the logger method instead of using the same log file name.

karuppasamy commented 10 years ago

Hi, Can you merge my pull request?

lulalala commented 10 years ago

Hi sorry I have been a bit busy lately. I am a bit concerned about three things:

Can you split the refactoring part and the enhancement part into separate commits?
I don't like refactoring that made if statement into one line in the get_path() method.
I think it makes more sense to specify file name instead of method name. What do you think?

Cheers~

karuppasamy commented 10 years ago

Yes, I'll split up the commit. May I know why you don't like to refactor the if statement? Anyway both are the same action, but with less code right? I can't understand your last statement. What are you meaning the method name?

lulalala commented 10 years ago

I guess the reason is a bit personal, the file naming is consisted of many appending steps, so the condition those occurs are very important, so I want to see them upfront. Personal preference really (like how I don't like "unless")

My third point: It makes more sense if we have optional parameter which specifies log filename. Currently in your commit, you use first argument as filename. I think the first argument should be use as method name instead, and user can use optional argument to specify different log filename.

karuppasamy commented 10 years ago

Yes I really understood your concerns. I'll change the things and recommit it soon as new pull request!. :)

karuppasamy commented 10 years ago

As requested, I have split my commits as a separate pull request https://github.com/lulalala/multi_logger/pull/9.