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

use composer command instead of editing json file #5829

Merged
merged 1 commit into from
Dec 19, 2015

Conversation

OskarStark
Copy link
Contributor

No description provided.

@javiereguiluz
Copy link
Member

Nice! 👍 I'd tweak it a bit and I'd remove the version number to leave it as follows:

$ composer require ircmaxell/password-compat

@xabbuh
Copy link
Member

xabbuh commented Oct 23, 2015

I agree. There's little risk a compatibility package releases a new major version containing BC breaks. Though if we wanted to keep a version, I would use ~1.0 or something similar instead.

@OskarStark
Copy link
Contributor Author

if the version is not necessary i am 👍

but i remembered this: sonata-project/SonataAdminBundle#2649 (diff)

@javiereguiluz
Copy link
Member

@OskarStark thanks for the info. I agree with @xabbuh's comments.

@OskarStark
Copy link
Contributor Author

so everything is fine or shall i change something?

@javiereguiluz
Copy link
Member

I'd use:

composer require ircmaxell/password-compat:~1.0

@OskarStark
Copy link
Contributor Author

fine for me, will change that

@OskarStark
Copy link
Contributor Author

Done, branch 2.3 is ok?

"ircmaxell/password-compat": "~1.0.3"
}
}
composer require ircmaxell/password-compat:~1.0
Copy link
Member

Choose a reason for hiding this comment

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

If I am not mistaken we always prefix shell commands with $.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

you are right, i fixed it

@wouterj
Copy link
Member

wouterj commented Oct 23, 2015

I actually do not like the version (note that the comment of me you linked was talking about a root package install, otherwise we often omit the version number).

However, if we want to use the version, we should change it to this to have Windows compability:

$ composer require ircmaxell/password-compat "~1.0"

@wouterj
Copy link
Member

wouterj commented Dec 17, 2015

@OskarStark can you please take a look at my comment and update this PR or comment?

@OskarStark
Copy link
Contributor Author

Will change that

@OskarStark OskarStark force-pushed the patch-9 branch 2 times, most recently from eebc487 to e171e68 Compare December 19, 2015 05:39
@OskarStark
Copy link
Contributor Author

done!

fine?

@wouterj wouterj merged commit a862a8c into symfony:2.3 Dec 19, 2015
wouterj added a commit that referenced this pull request Dec 19, 2015
…tark)

This PR was merged into the 2.3 branch.

Discussion
----------

use composer command instead of editing json file

Commits
-------

a862a8c use composer command instead of editing json file
wouterj added a commit that referenced this pull request Dec 19, 2015
@wouterj
Copy link
Member

wouterj commented Dec 19, 2015

Yes, perfect. Thanks! 🎄

@OskarStark
Copy link
Contributor Author

Thank you

@ghost
Copy link

ghost commented Dec 21, 2015

@OskarStark @wouterj It has been merged without the word composer, right? 079330e

@wouterj
Copy link
Member

wouterj commented Dec 21, 2015

Hmm, yes indeed. That has slipped through, can you please submit a PR to fix it?

OskarStark added a commit to OskarStark/symfony-docs that referenced this pull request Dec 22, 2015
@OskarStark
Copy link
Contributor Author

done @wouterj and good catch @JHGitty, thank you 👍

@OskarStark OskarStark deleted the patch-9 branch December 22, 2015 04:57
wouterj added a commit that referenced this pull request Dec 22, 2015
This PR was merged into the 2.3 branch.

Discussion
----------

minor #5829 Fix broken composer command

Commits
-------

73fa941 Update _ircmaxwell_password-compat.rst.inc
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants