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

Update codebase, bump Symfony components to ^5.4|^6.0, bump Monolog and more #111

Merged
merged 10 commits into from
Mar 13, 2023
Merged

Update codebase, bump Symfony components to ^5.4|^6.0, bump Monolog and more #111

merged 10 commits into from
Mar 13, 2023

Conversation

PAXANDDOS
Copy link
Contributor

@PAXANDDOS PAXANDDOS commented Feb 19, 2023

Q A
Bug fix? Yes
New feature? Yes (chores + Docker for development)
Breaking changes? Yes (PHP 7.4+, Symfony components 5.4.x, 6.x, Monolog 2.x, PHPUnit 9.x)
Issues resolved Fixes #104
Fixes #105
Fixes #108
Fixes #110

Changes

Environment
Added Docker support with PHP 7.4 or any desired version. PHP version can be set in docker-compose, default 7 (latest in 7.x). Pre-installed extensions: redis, hiredis, pcntl, apcu. Composer v2. Available Redis v7 via docker-compose. Usage explained [1]
Improved .editorconfig
Improved .gitattributes so code will always use LF
Fixes
Fixed speed:test command error (passing a command as a string when creating a Process instance)
Fixed various TypeErrors in commands (int code must be returned in execute() functions)
Fixed PHP deprecation of Serializable interface, now using magic methods
Fixed "Not defined" errors, mainly with Logger being used without a use statement
Fixed return types in various functions, which caused intellisense errors[2]
Fixed return code for Clear command (return was only inside the if block)
Fixed incorrect type in ConsoleProcessor
Fixed deprecation (null passed to strpos) in Util.php
Code improvements
Add types to class properties where possible (PHP 7.4 feature - Typed Properties). Some of the properties remain without the type because adding one will create execution errors[3]. All other properties - limited by PHP 7.4, they can store multiple types but declaring union types is only possible in PHP 8.0+, those properties only rely on DocBlocks
Add return and argument types to functions where possible (limited by the PHP version mentioned above), improve DocBlocks with correct types & union types
Add the final keyword to a class where it applies
Remove unused use statements and unused function parameters
Dependencies
Now application requires PHP version 7.4 and will work on versions up to 8.2 (see tests section below)
Updated Monolog to ^2.5 (Monolog ^2.5 works with PHP 7.2 or above), all deprecations fixed. Version ^3.0 is not allowed because has some major changes
Updated PHPUnit to ^9.0, code and configuration updated. Using version ^10.0 is not possible
Updated Symfony Console Process Yaml to version ^5.4 or ^6.0, which means 5.4 would be installed on PHP 7.4; 6.0 would be installed on PHP 8.0; 6.1 would be installed on PHP 8.1 and so on. Versions 3.x and 4.x dropped completely because components of 6.0 version require each other of version not lower than 5.4
Syntax improvements
Adopt short array syntax: Since PHP 5.4 the short array syntax [] may be used instead of array()
Multi-line arrays, arguments list, parameters list, and match expressions must have a trailing comma
List (array destructuring) assignment should be declared using the configured syntax on PHP >= 7.1
Heredoc/nowdoc content must be properly indented on PHP >= 7.3
Visibility MUST be declared on all properties and methods; abstract and final MUST be declared before the visibility; static MUST be declared after the visibility
Use null coalescing operator ?? where possible on PHP >= 7.0
Add void return type to functions with missing or empty return statements, but priority is given to return annotations on PHP >= 7.1
Anonymous functions with one-liner return statements must use arrow functions
Master functions shall be used instead of aliases
Adjust spacing around the colon in return type declarations and backed enum types
All instances created with new keyword must be followed by braces
Equal sign in declare statement should be surrounded by spaces or not the following configuration

Tests

During tests I used all possible methods:

  • Running composer test
  • Testing all options in Web UI
  • Testing all commands available under bin/resque except socket commands
PHP version Dependencies Status
PHP 7.4.33 Predis v1.1.10
Monologv2.9.1
Symfony Console v5.4.19
Symfony Process v5.4.19
Symfony Yaml v5.4.19
PHP 8.0.28 Predis v1.1.10
Monologv2.9.1
Symfony Console v6.0.19
Symfony Process v6.0.19
Symfony Yaml v6.0.19
PHP 8.1.16
PHP 8.2.3
Predis v1.1.10
Monologv2.9.1
Symfony Console v6.2.5
Symfony Process v6.2.5
Symfony Yaml v6.2.5

Post-word

Number Description
[1] - Docker of the latest version must be installed.
- Specify desired PHP version in docker-compose.yml
- Run docker compose up -d at the root of the project
- Run docker exec -it resque sh to enter the container
- Application is ready to use
* start hosting Web UI cd examples php -S 0.0.0.0:8000 and open the browser on localhost:8000, port-forwarding specified in docker-compose.yml
* to use Redis from the container, change 127.0.0.1 in config to redis, the hostname is the name of the service in docker-compose.yml
[2] For example, Job::push() had an annotation to return a string, when in fact it was returning Job instance. This made me confused when I was setting queue for my application
[3] When type is defined, properties like, for example, Resque\Job::$worker will cause execution error src\Resque\Job.php on line 262 where setStatus() is called. At this moment $worker is not defined and without Typed Property will be mixed for the compiler. When type is defined, throws an error: typed property $worker must not be accessed before initialization

And that's it. Also wanted to say that I like this library a lot and hoped to help to keep it maintained. Couldn't find many of the non-abandoned packages for the Redis queue out there, and yours has almost all of the features I needed!
I've made a lot of changes, and hope documented them well; literally spent my whole weekend on this

- Added Docker support with PHP 7.4 (redis, hiredis, pcntl, apcu extensions; composer v2) and Redis v7 via docker-compose
- Increased minimun required PHP version to 7.4 (not LTS, latest in 7.x)
- Dropped support for v3.x Symfony components, improved composer versioning
- Upgraded PHPUnit to 9.x (v10 is latest but acting weird), updated code respectfully
- Improved .editorconfig
- Improved .gitattributes so code will always use LF
- [PHPUnit] Set return type of base TestCase methods: From the [PHPUnit 8 release notes][1], the `TestCase` methods below now declare a `void` return type:
  - `setUpBeforeClass()`
  - `setUp()`
  - `assertPreConditions()`
  - `assertPostConditions()`
  - `tearDown()`
  - `tearDownAfterClass()`
  - `onNotSuccessfulTest()`
- [PHP] Adopt short array syntax: Since PHP 5.4 the short array syntax `[]` may be used instead of `array()`
- Updated php-cs-fixer configuration for latest php-cs-fixer version and now using it from "require-dev"
- Formatted codebase with php-cs-fixer (using existing configuration)
- Add types to class properties where possible (PHP 7.4 feature - Typed Properties)
- Add and fix function return and argument types where possible (limited by PHP version), improve doctypes with correct types & union types
- Fix all "Not defined" errors, mainly with Logger being used but not imported
- Add "final" keyword to a class where it applies
- Remove unused "use" statements, unused function parameters
- Fix PHP8+ deprecation of Serializable interface
- Bump Symfony components to ^5.4|^6.0
- Improve tests types
- Fix incorrect type in ConsoleProcessor
- Fix deprecation (null to strpos) in Util.php
- Bump Resque version to 3.2.0
- Allow passing PHP version as argument in dockerfile/docker-compose
- Multi-line arrays, arguments list, parameters list and match expressions must have a trailing comma
- List (array destructuring) assignment should be declared using the configured syntax on PHP >= 7.1
- Heredoc/nowdoc content must be properly indented on PHP >= 7.3
- Visibility MUST be declared on all properties and methods; abstract and final MUST be declared before the visibility; static MUST be declared after the visibility
- Use null coalescing operator ?? where possible on PHP >= 7.0
- Add void return type to functions with missing or empty return statements, but priority is given to @return annotations on PHP >= 7.1
- Anonymous functions with one-liner return statement must use arrow functions
- Master functions shall be used instead of aliases
- Adjust spacing around colon in return type declarations and backed enum types
- All instances created with new keyword must (not) be followed by braces
- Equal sign in declare statement should be surrounded by spaces or not following configuration
@xelan
Copy link
Collaborator

xelan commented Feb 20, 2023

Wow, awesome work @PAXANDDOS, thank you very much 🎉! As there are quite some breaking changes (higher PHP requirement, Symfony and Monolog version bumps) I'd create a new major version v4, but nevertheless still maintain v3 for compatibility with legacy systems.

I'll review and test your new version, and apply some of your changes (parameter and return types where possible, new array syntax, code style improvements) to the upcoming v3 version 3.2.0 as well.

@PAXANDDOS
Copy link
Contributor Author

I'm so glad it was helpful! Although I didn't think of it as a major release at the start, for example, the difference between PHP 7.1 (your current minimum) and 7.4 is not so big. But yeah, drops of Monolog and Symfony components make me feel the need for that. I just hope to see that v4 up quite soon 😄
As for 3.2.0, keep in mind that you might want to drop support for Symfony 3.x components. Because, for example, Process won't work on both of them (for 3.x constructor accepts string, for 4.x+ constructor accepts array)

@xelan
Copy link
Collaborator

xelan commented Feb 20, 2023

I'm so glad it was helpful! Although I didn't think of it as a major release at the start, for example, the difference between PHP 7.1 (your current minimum) and 7.4 is not so big. But yeah, drops of Monolog and Symfony components make me feel the need for that.

Yep, that's why a major version bump should be done IMHO 👍

I just hope to see that v4 up quite soon smile As for 3.2.0, keep in mind that you might want to drop support for Symfony 3.x components. Because, for example, Process won't work on both of them (for 3.x constructor accepts string, for 4.x+ constructor accepts array)

For the Symfony Process compatibility I've already written a small factory class, which can be integrated here as well:

class ProcessFactory
{
    public function createProcess($commandLine)
    {
        $sf42orHigher = method_exists(Process::class, 'fromShellCommandline');

        return $sf42orHigher ? Process::fromShellCommandline($commandLine) : new Process($commandLine);
    }
}

Furthermore, the speedtest and its symfony/process requirement are optional, so I wouldn't drop SF3.4 compatibility just for the sake of it.

@PAXANDDOS
Copy link
Contributor Author

May I ask you how do you plan on releasing 2 versions simultaneously? I'm not familiar with the process thus I'm quite curious--
Would be master branch for v4 (for the latest release) l and a separate branch (3.x) for the older one? Or would you go the Symfony way: no master branch at all, separate branch for each version (3.0 3.1 3.2) and the latest release is the default branch? Also, I'd suggest making releases and not just tags (releases include tags)

@xelan
Copy link
Collaborator

xelan commented Feb 22, 2023

May I ask you how do you plan on releasing 2 versions simultaneously? I'm not familiar with the process thus I'm quite curious-- Would be master branch for v4 (for the latest release) l and a separate branch (3.x) for the older one? Or would you go the Symfony way: no master branch at all, separate branch for each version (3.0 3.1 3.2) and the latest release is the default branch? Also, I'd suggest making releases and not just tags (releases include tags)

I'd create a v3 branch for the older release and maintain v3 at least until the end of 2023, as we still have legacy systems which cannot be upgraded to PHP 7.4 at the moment. The master branch will get the latest v4 updates.

@PAXANDDOS
Copy link
Contributor Author

Hey @xelan! Hope you're doing great
How's the progress with PR going? Did you have an opportunity to test it?

I'm actually texting you to not merge the PR just yet-- Maybe you can create a dev branch for 4.x or simply a 4.0 branch based on the master? I'd change the target branch of this PR to the new branch. The reason is that I'd like to make even more PRs with fixes, docs, and even new feature(s)-- don't want to stack them up in this PR

@xelan
Copy link
Collaborator

xelan commented Mar 1, 2023

Hi @PAXANDDOS! The usage of a v4 branch for now sounds like a great suggestion. At the moment I'm quite busy, so unfortunately I could not test the changes of your PR yet.

@PAXANDDOS
Copy link
Contributor Author

That's unfortunate... Been using a downgraded Symfony Console in my application for a while now and it creates a bunch of conflicts when I'm trying other packages 😢

@xelan
Copy link
Collaborator

xelan commented Mar 13, 2023

Hi @PAXANDDOS! Thanks again for your PR. Your changes look good so far, I've created a v4 branch. Could you please change the target of the PR to that branch?

Best regards
Andreas

@PAXANDDOS PAXANDDOS changed the base branch from master to v4 March 13, 2023 12:22
@PAXANDDOS
Copy link
Contributor Author

Hi @xelan! Done! And I started to lose hope... 😅 I will try to compose some more PRs, though I might have forgotten certain things I wanted to improve

@xelan xelan merged commit 48c81ea into mjphaynes:v4 Mar 13, 2023
@xelan xelan mentioned this pull request Oct 30, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants