Skip to content
This repository has been archived by the owner on Mar 23, 2023. It is now read-only.

Added --autoloader/-a option to fresque start command #37

Open
wants to merge 13 commits into
base: master
Choose a base branch
from
Open

Added --autoloader/-a option to fresque start command #37

wants to merge 13 commits into from

Conversation

shadyb
Copy link

@shadyb shadyb commented Jul 22, 2014

Hey,

Great project. I noticed the start option did not have any ability to override the include option in the config file so I added it to the cli. Please review and let me know what you think.

Regards

@wa0x6e
Copy link
Owner

wa0x6e commented Jul 23, 2014

Did you take a look at https://github.com/kamisama/Fresque/blob/master/lib/Fresque.php#L194 ? Is it different ?

@wa0x6e wa0x6e self-assigned this Jul 23, 2014
@shadyb
Copy link
Author

shadyb commented Jul 24, 2014

Yes, that is where it was inspired from. Basically I needed the ability to add different bootstraps for different webapps on the same server so I used the --autoloader option from the test command and added it to the start command.

There is one other deficiency, I cannot pass custom arguments to the bootstrap. I would like to propose something like --arguments APPLICATION=staging FOO=bar
At the moment I have hard coded an --environment option but that is too restrictive. What are your thoughts ?

@wa0x6e
Copy link
Owner

wa0x6e commented Jul 24, 2014

You can try using environment variables.

APPLICATION=staging FOO=bar fresque start

@wa0x6e
Copy link
Owner

wa0x6e commented Jul 24, 2014

And if you have different webapps, why don't you put these settings in a fresque .ini file, and have one .ini file per webapp ?

@shadyb
Copy link
Author

shadyb commented Jul 24, 2014

Will try that and see what happens.

Because I am using jenkins and it is far easier to pass on the variable via cli than to have to grab the var, generate an entirely new ini file when all you want to do is override one variable.

@shadyb
Copy link
Author

shadyb commented Jul 24, 2014

The suggested approach to setup the environment variables doesn't work. You are creating a new command, under a different user which spawns off to be a new process and the environment vars that are set will only affect the existing command. I recommend the --arguments option or some option in fresque.ini that'll allow you to initialise the environment variables when constructing the $cmd. Let me know what you think, if you think its a good idea, I'll whip something up.

Also, do let me know what you want to do with the --autoloader argument for the start command.

@wa0x6e
Copy link
Owner

wa0x6e commented Jul 24, 2014

For the environment variables in the config, I'd like something like https://github.com/kamisama/Cake-Resque/blob/master/Config/config.php#L135, to maintain compatibility between these 2 tools.

Maybe --env is a more appropriate name ?

I'm still reviewing the --autoloader, it's been a while since I last touched fresque, and even php ...

I propose that you separate the environment variables feature from the --autoloader, and submit another PR.

@shadyb
Copy link
Author

shadyb commented Jul 25, 2014

I agree, --env is better than --arguments. Not sure what you mean by compatibility here; do you mean in terms of naming conventions or the code itself ? I haven't seen anything specific to cakephp in fresque.

I agree, I will remove the --environment option; let's get the --autoloader vetted and out and then focus on the --env cmd.

…vironment (if specified)"

This reverts commit 6aa2fa3.
@shadyb
Copy link
Author

shadyb commented Jul 30, 2014

Any luck with this ?

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants