Skip to content

Make artisan executable #2027

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
May 28, 2013
Merged

Conversation

nils-werner
Copy link
Contributor

To make it easier and more obvious how to use artisan I would recommend making the file executable.

Right now, artisan can only be executed using php artisan; after merging this PR it will also be possible to run ./artisan directly (that's what the first line in the file is for anyways).

@crynobone
Copy link
Member

See #1597

@nils-werner
Copy link
Contributor Author

Interesting that everybody agreed on "eh... there must be a reason" although none was ever mentioned :D

@baskan
Copy link

baskan commented May 28, 2013

Well, I do think this has to be done by the user who installs laravel.

@nils-werner
Copy link
Contributor Author

But why?

@baskan
Copy link

baskan commented May 28, 2013

well, if you estimate that your laravel install out of the box should offer this hashbang + chmod feature you'll reference command tool with ./artisan but referencing like this is not compatible with non-unix enviroments. But php artisan command is OS-agnostic and I think its better to stay that way so we can reference its commands the same.

@nils-werner
Copy link
Contributor Author

You'd be referencing it by php artisan or ./artisan, both are valid; so it would still be OS-agnostic but also offer "progressive enhancement" for bash-shells

Secondly, and more importantly, it would add to the readability of the code: A non-executable file with no file ending is not helpful at all. You would have to read the docs in order to find out how to use that thing. An executable file on the other hand makes it immediately obvious that this file should be run from the commandline.

taylorotwell added a commit that referenced this pull request May 28, 2013
@taylorotwell taylorotwell merged commit 443fc9d into laravel:develop May 28, 2013
@nils-werner nils-werner deleted the executable branch May 28, 2013 13:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants