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

[5.2] UPD composer files, phpseclib/bcmath_compat 2.0.3 #44036

Merged
merged 8 commits into from
Sep 11, 2024

Conversation

tkuschel
Copy link
Contributor

@tkuschel tkuschel commented Sep 8, 2024

Pull Request for Issue #44028 .

Updated composer files, b/c of fixed phpseclib/bcmath_compat#6

Summary of Changes

phpseclib/bcmath_compat from 2.0.2 to 2.0.3

The following 3 packages have also been updated as a side effect as expected:
maximebf/debugbar v1.22.3 to v1.22.4
friendsofphp/php-cs-fixer v3.59.3 to v3.64.0
phpstan/phpstan 1.12.0 to 1.12.2

Actual result BEFORE applying this Pull Request

bcmath_compat:
The exception ArgumentCountError gives wrong given parameter length. see phpseclib/bcmath_compat#6
for more details.

Expected result AFTER applying this Pull Request

Fixed

Link to documentations

  • [X ] No documentation changes for docs.joomla.org needed
  • [X ] No documentation changes for manual.joomla.org needed

@tkuschel
Copy link
Contributor Author

tkuschel commented Sep 8, 2024

What did I do wrong here that the build test "continuous-integration/dron/pr with phpcs failed?
Is it correct that all files that are in the log file of the cs-fixer must be adjusted; so here in this case it is the sprintf which should be written as \sprintf, according to the phpcs.

@tkuschel tkuschel changed the title UPD composer files, phpseclib/bcmath_compat 2.0.3 [5.2] UPD composer files, phpseclib/bcmath_compat 2.0.3 Sep 8, 2024
@richard67
Copy link
Member

richard67 commented Sep 8, 2024

What did I do wrong here that the build test "continuous-integration/dron/pr with phpcs failed? Is it correct that all files that are in the log file of the cs-fixer must be adjusted; so here in this case it is the sprintf which should be written as \sprintf, according to the phpcs.

@tkuschel If you check the changes in the composer.lock file from your PR, you will see that it doesn't only update the phpseclib/bcmath_compat dependency but also others, including the code style checker. I think that's the reason why it complains about unrelated files now. Maybe you have run a composer update (without any option) instead of composer update phpseclib/bcmath_compat for updating just that dependency?

@tkuschel
Copy link
Contributor Author

tkuschel commented Sep 8, 2024

Thank you for the detailed description, I have corrected this.

@brianteeman
Copy link
Contributor

It would have been better if you did what @richard67 said and ONLY updated bcmath with this PR.

Other changes such as phpcs and changing the sprintf should be in their own pull requests.

@tkuschel
Copy link
Contributor Author

tkuschel commented Sep 8, 2024

Other changes such as phpcs and changing the sprintf should be in their own pull requests.

That's a good point. I will pick this apart. I didn't imagine that the composer update would cause such work :-) See #44037

@richard67
Copy link
Member

@tkuschel You haven't reverted yet the unrelated changes of other dependencies.

@tkuschel
Copy link
Contributor Author

tkuschel commented Sep 8, 2024

@tkuschel You haven't reverted yet the unrelated changes of other dependencies.

Had no time, now it looks good. I had dependencies with other local branches.

@Hackwar Hackwar merged commit 203500f into joomla:5.2-dev Sep 11, 2024
0 of 2 checks passed
@Hackwar
Copy link
Member

Hackwar commented Sep 11, 2024

Thank you for your contribution @tkuschel!

@Hackwar Hackwar added this to the Joomla! 5.2.0 milestone Sep 11, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants