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.

Sign up for GitHub

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

Jump to bottom

Add workflows and scripts, bump symfony/console, require nextcloud/co… #421

Merged
merged 10 commits into from
Aug 4, 2022
Merged

Add workflows and scripts, bump symfony/console, require nextcloud/co… #421

merged 10 commits into from
Aug 4, 2022

Conversation

come-nc
Copy link
Collaborator

@come-nc come-nc commented Jul 25, 2022

…ding-standard

Signed-off-by: Côme Chilliet come.chilliet@nextcloud.com

@come-nc
Add workflows and scripts, bump symfony/console, require nextcloud/co…
df28038 
…ding-standard

Signed-off-by: Côme Chilliet <come.chilliet@nextcloud.com>
@come-nc come-nc self-assigned this Jul 25, 2022
@come-nc
Copy link
Collaborator Author

come-nc commented Jul 25, 2022

Is the vendor directory versioned on purpose? Is it ok to bump symfony/console dep like I did?

@kesselb
Copy link
Contributor

kesselb commented Jul 27, 2022

Is the vendor directory versioned on purpose?

I don't know. Maybe related to the build script for the phar file.

Is it ok to bump symfony/console dep like I did?

Looks good to me. Some dev dependencies are new. Could we scope them like nextcloud/server#28507 to not add more dependencies to git?

@MorrisJobke MorrisJobke removed their request for review July 28, 2022 07:27
@come-nc
Copy link
Collaborator Author

come-nc commented Jul 28, 2022

Looking into the updater.phar that I get after a make updater.phar I think we are fine because the namespaces listed in the phar do not include any dev dependency.
The new updater.phar is 30K larger than master, but this is consistent with the dependency bump of symfony/console which is also larger.

@come-nc
Copy link
Collaborator Author

come-nc commented Jul 28, 2022

Pro tip: ./box info -l updater.phar can be used to list content and sizes of the updater, and then diff that with master to see what actually changed in the phar. Changes seems to be correct.

@come-nc come-nc force-pushed the fix/add-workflows branch from bb7241b to 0308477 Compare July 28, 2022 14:29
@come-nc
Copy link
Collaborator Author

come-nc commented Jul 28, 2022

I’m cancelling trying to update the tested version numbers to supported versions because the signatures are hardcoded and it’s getting too complicated. So for now we’ll keep testing the updater with 19-21 and master.

@come-nc come-nc force-pushed the fix/add-workflows branch 2 times, most recently from 0006c78 to 6d49b89 Compare July 28, 2022 14:55
@come-nc
Copy link
Collaborator Author

come-nc commented Jul 28, 2022

Moved the testing from drone to github workflow to be able to setup any PHP version, everything is green again, except master but it should turn green tomorrow thanks to https://github.com/nextcloud-gmbh/release-script/pull/58
I need someone with admin access to remove the drone job from expected jobs.

Apart from that and a bit of squashing I think we are good to merge.

I will open other PR to maybe add psalm and in anycase fix the 8.1 warnings.

come-nc and others added 5 commits August 1, 2022 10:19
@come-nc
Remove PHP 7.3 from CI
5fc8e82 
Signed-off-by: Côme Chilliet <come.chilliet@nextcloud.com>
@kesselb @come-nc
Scope nextcloud/coding-standard
515841b 
Signed-off-by: Daniel Kesselberg <mail@danielkesselberg.de>
@come-nc
Use box compile instead of deprecated box build
9b1d9d9 
Signed-off-by: Côme Chilliet <come.chilliet@nextcloud.com>
@come-nc
composer run cs:fix
5bda9e4 
Signed-off-by: Côme Chilliet <come.chilliet@nextcloud.com>
@come-nc
Add github workflows for testing
8a873e6 
Signed-off-by: Côme Chilliet <come.chilliet@nextcloud.com>
@come-nc come-nc force-pushed the fix/add-workflows branch from 83fadb0 to 44fbdf7 Compare August 1, 2022 08:23
@come-nc
Testing all versions through github workflows
a93852d 
Exclude testing 19 and 20 on PHP 8.0
Test cli on 7.4 as it tests 19 and 20

Signed-off-by: Côme Chilliet <come.chilliet@nextcloud.com>
@come-nc come-nc force-pushed the fix/add-workflows branch from 44fbdf7 to a93852d Compare August 2, 2022 07:51
@come-nc
Master version is now at 25
587c330 
Signed-off-by: Côme Chilliet <come.chilliet@nextcloud.com>
@come-nc
Add summaries for test-stable and test-master workflows
9ee5aa0 
Signed-off-by: Côme Chilliet <come.chilliet@nextcloud.com>
kesselb
kesselb approved these changes Aug 2, 2022
View reviewed changes
Copy link
Contributor

@kesselb kesselb left a comment

Choose a reason for hiding this comment

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

LGTM 👍

@come-nc
Ignore vendor-bin folder in PHP code sniffer
86df76f 
Signed-off-by: Côme Chilliet <come.chilliet@nextcloud.com>
@come-nc come-nc requested review from skjnldsv and CarlSchwan August 2, 2022 15:30
@PVince81 PVince81 merged commit 3f953af into master Aug 4, 2022
@delete-merged-branch delete-merged-branch bot deleted the fix/add-workflows branch August 4, 2022 08:30
@blizzz
Copy link
Member

blizzz commented Aug 4, 2022

do we need backports?

@skjnldsv skjnldsv mentioned this pull request Aug 11, 2022
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@kesselb kesselb kesselb approved these changes

@juliusknorr juliusknorr Awaiting requested review from juliusknorr

@blizzz blizzz Awaiting requested review from blizzz

@ChristophWurst ChristophWurst Awaiting requested review from ChristophWurst

@skjnldsv skjnldsv Awaiting requested review from skjnldsv

@CarlSchwan CarlSchwan Awaiting requested review from CarlSchwan

Assignees

@come-nc come-nc

Labels
4. to release
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@come-nc @kesselb @blizzz @PVince81