search

bentglasstube / App-MPDJ

Automated DJ for MPD
Other
0 stars 1 forks source link

Configuration File Support #6

Closed john- closed 10 years ago

john- commented 10 years ago

The approach used for adding config file support (use of AppConfig) also changed how command line options are defined internally (user interface remains the same). I looked at different options for reading from config files but with its single approach for both config and command line option definition AppConfig one out. I could have missed another option that would work better so feel free to point something out that you think might.

Here is sample config:

nodaemon

paths to stuff

music-path=/ calls-path=incoming/calls

behavior

before=2 after=3 calls-freq=4800

logging

syslog=debug conlog=debug

bentglasstube commented 10 years ago

I have merged this in but I changed how the precedence works a bit.

With your implementation the ~/.mpdjrc file was taking precedence over the file given on the command line, which I thought was undesirable. I have changed it so that if something is given on the command line, neither of the default configuration files will be used.

I also did a bunch of various cleanups so that will probably making merging master back onto any branches you have a nightmare. You will want to grab master again before starting anything new I'd think. Sorry if this is a big pain.

I am not pushing this to CPAN just yet as I haven't had time to test it all (I really need to get testing up to snuff), but I wanted to get your changes in before I forgot about them for a week again.

If you have any issues with it, please don't hesitate to let me know.

bentglasstube commented 10 years ago

Oh also, I got everything all messed up with the repo's history somehow because I was being a noob at git so you might have some strange issues with pulling. Again, sorry, it's been one of those days >_<

john- commented 10 years ago

On 03/06/14 21:02, Alan Berndt wrote:

Oh also, I got everything all messed up with the repo's history somehow because I was being a noob at git so you might have some strange issues with pulling. Again, sorry, it's been one of those days >_<

— Reply to this email directly or view it on GitHub https://github.com/bentglasstube/App-MPDJ/pull/6#issuecomment-36963774.

No problem. I had a lot of problems on my end getting things merged in. I wouldn't be be surprised if something I did made it harder for you.

john- commented 10 years ago

On 03/06/14 20:58, Alan Berndt wrote:

I have merged this in but I changed how the precedence works a bit.

With your implementation the ~/.mpdjrc file was taking precedence over the file given on the command line, which I thought was undesirable. I have changed it so that if something is given on the command line, neither of the default configuration files will be used.

That make sense. I did not think it through.

I also did a bunch of various cleanups so that will probably making merging master back onto any branches you have a nightmare. You will want to grab master again before starting anything new I'd think. Sorry if this is a big pain.

I am not pushing this to CPAN just yet as I haven't had time to test it all (I really need to get testing up to snuff), but I wanted to get your changes in before I forgot about them for a week again.

For sure work with it a while. I will get my repo ready for any changes you think I need to make.

john- commented 10 years ago

It was a good experience for me seeing some of the changes you made to the code. I learned a lot! Hopefully that will improve the next pull request I make :)

bentglasstube commented 10 years ago

Most of it was just clean up. There was nothing wrong with your request really. I am always gals for the contributions.

Alan Berndt On Mar 7, 2014 8:57 AM, "john-" notifications@github.com wrote:

It was a good experience for me seeing some of the changes you made to the code. I learned a lot! Hopefully that will improve the next pull request I make :)

— Reply to this email directly or view it on GitHubhttps://github.com/bentglasstube/App-MPDJ/pull/6#issuecomment-37037053 .