Skip to content

chore: also test with PHP 8.3#1435

Closed
Croydon wants to merge 7 commits intopulsejet:masterfrom
Croydon:ci_nc_31
Closed

chore: also test with PHP 8.3#1435
Croydon wants to merge 7 commits intopulsejet:masterfrom
Croydon:ci_nc_31

Conversation

@Croydon
Copy link
Contributor

@Croydon Croydon commented Apr 3, 2025

Why?

Test with all current NC versions + all PHP versions that are supported by all NC versions

https://github.com/nextcloud/server/wiki/Releases-and-PHP-versions

Croydon added 4 commits April 3, 2025 20:49
Copied from https://github.com/nextcloud/coding-standard/blob/af952f469a320200b89ab529205c63a861d35308/src/Config.php

I basically added all rules that php-cs-fixer wanted to apply to the files with the previous config AND that have an explicit rule in the NC coding standards repository.

The goal was to being as close to the NC coding standard as possible without entirely coping it (for now).
@Croydon Croydon marked this pull request as draft April 3, 2025 19:39
@Croydon
Copy link
Contributor Author

Croydon commented Apr 3, 2025

All I wanted was to extend test coverage to current PHP and NC versions, but in order to get PHP 8.3 support, I needed to update php-cs-fixer from 3.35 to 3.75.

In those updates, php-cs-fixer has added an awful amount of new rules, so this ended in a formatting war.

I don't know how opinioned you are about formatting @pulsejet, and what your preferences are. Personally, I don't care that much when tools do the formatting job anyway.

I would suggest to just adopt whatever https://github.com/nextcloud/coding-standard/blob/master/src/Config.php is doing to have consistency with other Nextcloud code.

I will probably split out NC 31 CI coverage to another pull request to get that part of it faster in a mergeable state.

@Croydon
Copy link
Contributor Author

Croydon commented Apr 3, 2025

I have splitted out test coverage for NC 31: #1436

@Croydon Croydon changed the title chore: also test with NC v31 and php-lint with 8.3 chore: also test with PHP 8.3 Apr 3, 2025
@arnowelzel
Copy link
Contributor

arnowelzel commented Jul 30, 2025

The standard for functions, classes and methods is to put the opening braces (curly brackets) in separate lines and not next to it. Also see the examples at https://www.php-fig.org/psr/psr-1/ and https://www.php-fig.org/psr/psr-12/.

Example - this is common practice:

class MyClass
{
    function myMethod()
    {
        $x = doSomething();
        if ($x < 5) {
            doSomethingElse();
        }
    }
}

But not this, because it is then harder to recognize the start of a method compared to the rest of the code:

class MyClass {
    function myMethod() {
        $x = doSomething();
        if ($x < 5) {
            doSomethingElse();
        }
    }
}

@Croydon
Copy link
Contributor Author

Croydon commented Jul 30, 2025

Changes in this PR are either merged at this point or are no longer needed.

Closing in favor of #1502

@Croydon Croydon closed this Jul 30, 2025
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.

2 participants