-
-
Notifications
You must be signed in to change notification settings - Fork 4.7k
chore: move behat dependencies to vendor-bin #56718
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
Conversation
5e71d01 to
ab247d6
Compare
ab247d6 to
cc3cfac
Compare
build/integration/composer.json
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This also breaks some scripting we have here and there, e.g. https://github.com/nextcloud/docker-ci/blob/f5a19c0d41e5a7a615ef97cf3b4b0a8afa5ebc20/integration-php8.1/Dockerfile#L5
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That already breaks because there is no /tests/acceptance folder.
I also do not understand what that should do:
- clone server
- install dependencies
- remove everything ❓
So from the current state where tests/acceptance do not exist and it works it seems it will also work with this changes here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yeah for php 8.3 that one was removed:
https://github.com/nextcloud/docker-ci/blob/master/integration-php8.3/Dockerfile
but we should basically align this then.
but I once again got confused by our weird naming. it's not the continuous-integration-php8.3 but the continuous-integration-integration-php8.3 🙈
The only place that uses that is the server run intergration tests in docker, but yeah basically rebuilding that image will be broken, but then I don't care for the past.
We just should fix master for the latest php version
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The only place that uses that is the server run intergration tests in docker, but yeah basically rebuilding that image will be broken, but then I don't care for the past.
Shall we then just drop the image completely and just use the normal image in server?
Because we can do the same in the server script.
782c7a0 to
ee4133d
Compare
nickvergessen
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The only negative thing is that you need to install composer deps in server root, which makes lib/composer/* dirty again, but it's the same for psalm, cs and others so I guess we are used to it by now
So we do not have 3 locatations to look for PHP dependencies. Signed-off-by: Ferdinand Thiessen <opensource@fthiessen.de>
Signed-off-by: Ferdinand Thiessen <opensource@fthiessen.de>
ee4133d to
e0e2d0f
Compare
Summary
So we do not have 3 locatations to look for PHP dependencies.
Checklist
3. to review, feature component)stable32)