Skip to content
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

Put doctrine-migrations into including project's vendor/bin directory #311

Merged
merged 4 commits into from
Jul 29, 2015

Conversation

dotEvan
Copy link

@dotEvan dotEvan commented Jul 21, 2015

Enables a project to include doctrine/migrations and then to run any migration commands by running

./vendor/bin/doctrine-migrations

from the main project's directory.

@mikeSimonson
Copy link
Contributor

Thanks @dotEvan .

Don't we need to set as executable only the doctrine-migrations file and not the php one ?

},
"bin": [
"bin/doctrine-migrations",
"bin/doctrine-migrations.php"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

bin/doctrine-migrations.php should not be there. Only the actual bin is needed

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@stof To be honest, I only use the version without the .php. However, I was being consistent with the way this was done in https://github.com/doctrine/doctrine2/blob/master/composer.json

I didn't want to assume that people didn't also want to be able to do

php ./vendor/bin/doctrine-migrations.php

if they wanted to.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

And this is a mistake in the ORM (it was reverted in DBAL but I missed the fact that the ORM included it too)

@stof
Copy link
Member

stof commented Jul 21, 2015

@mikeSimonson all files listed in the bin section of composer must be executable (otherwise Composer will make them executable and then the next update will tell you that the package has some changes).
But the mistake is listing the PHP file in this section in the first place

@tejerka
Copy link

tejerka commented Jul 28, 2015

+1

@mikeSimonson
Copy link
Contributor

@dotEvan Can you make that change in your PR ?

Revert the change of permission on bin/doctrine-migrations.php and remove bin/doctrine-migrations.php from the bin section of the composer.json ?

@dotEvan
Copy link
Author

dotEvan commented Jul 28, 2015

@mikeSimonson Done.

@dotEvan dotEvan changed the title Put doctrine-migrations{.php} into including project's vendor/bin directory Put doctrine-migrations into including project's vendor/bin directory Jul 28, 2015
mikeSimonson added a commit that referenced this pull request Jul 29, 2015
Put doctrine-migrations into including project's vendor/bin directory
@mikeSimonson mikeSimonson merged commit aa34a11 into doctrine:master Jul 29, 2015
@mikeSimonson
Copy link
Contributor

@dotEvan Thanks

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.

None yet

5 participants