Skip to content

Conversation

exaby73
Copy link
Collaborator

@exaby73 exaby73 commented Jan 29, 2025

Re opening this PR. I believe the HTTP removal PR (#244) was merged first, before merging #255. And since these were stacked PRs, #244 merged to main, then #255 merged to feat/remove-http but after it was merged to main so the code in #255 did not reach main at all

@exaby73 exaby73 changed the title feat: Remove formatter feat!: Remove formatter Jan 29, 2025
@exaby73
Copy link
Collaborator Author

exaby73 commented Jan 29, 2025

So the PHP CS Fixer package is not happy with the PHP version in CI so I updated it to latest. On doing that though, it suddenly started removing some PHPDoc params. See diff below

     /**
-     * @param list<UnmanagedTransaction> $tsxs
-     *
      * @return list<UnmanagedTransaction>
      */
     private function addTransactionOrRun(int $i, array $tsxs, int $retriesLeft = 10): array

I tracked it down to kubawerlos/php-cs-fixer-custom-fixers's PhpdocNoSuperfluousParamFixer fixer which is configured in our configuration for php-cs-fixer config. @transistive do you think we should remove PhpdocNoSuperfluousParamFixer from our configs? I'm afraid we'll be losing some type safety if we keep it and upgrade php-cs-fixer

@transistive transistive force-pushed the feat/remove-formatter branch from 4d1d3c3 to 4579b9f Compare February 13, 2025 06:43
Copy link
Collaborator

@transistive transistive left a comment

Choose a reason for hiding this comment

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

Great stuff. I checked the code diff locally withoutthe cs changes and it looked great. I just pulled back in the tests from the OGM Formatter as they still apply in the summarized resultFormatter

@transistive transistive force-pushed the feat/remove-formatter branch 3 times, most recently from 54b4c80 to 1d2fadc Compare February 13, 2025 09:51
@transistive transistive force-pushed the feat/remove-formatter branch from 1d2fadc to be6a56c Compare February 13, 2025 09:54
@transistive transistive merged commit 47d972e into main Feb 13, 2025
12 checks passed
@transistive transistive deleted the feat/remove-formatter branch March 9, 2025 11:26
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