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

Require pimple not only as a dev dependency but also production since ckfinder needs it #2224

Merged
merged 1 commit into from
Sep 1, 2017

Conversation

carakas
Copy link
Member

@carakas carakas commented Aug 31, 2017

Type

  • Critical bugfix

Pull request description

Pimple was added as dev dependency while we also need it in production for ckfinder

@carakas carakas added this to the 5.0.3 milestone Aug 31, 2017
@mention-bot
Copy link

@carakas, thanks for your PR! By analyzing the history of the files in this pull request, we identified @WouterSioen, @jeroendesloovere and @tijsverkoyen to be potential reviewers.

Copy link
Member

@WouterSioen WouterSioen left a comment

Choose a reason for hiding this comment

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

I don't get the "pimple was added as a dev dependency" because it's not shown in the require-dev part, but seems good to me.

@StijnVrolijk
Copy link
Contributor

StijnVrolijk commented Sep 1, 2017

@WouterSioen either one of the packages requires pimple as a dev requirement incorrectly or @carakas made a booboo. I'm guessing the first since the pimple package was in fact in the lock file.

@WouterSioen
Copy link
Member

still a booboo :D

@carakas
Copy link
Member Author

carakas commented Sep 1, 2017

@WouterSioen nobody is perfect :) it is indeed something I didn't notice, but I'm also the one fixing it :)

@carakas carakas merged commit 14fd4f3 into forkcms:master Sep 1, 2017
@carakas carakas deleted the fix-pimple-in-dev-requirement branch September 1, 2017 07:13
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