Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Appearance settings

Remove sensio if possible #466

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

Merged
merged 10 commits into from
Dec 24, 2021
Merged

Remove sensio if possible #466

merged 10 commits into from
Dec 24, 2021

Conversation

TomasVotruba
Copy link
Member

@TomasVotruba TomasVotruba commented Dec 23, 2021

The only feature we use from the sensio framework bundle extra is entity resolution. It will be added to Symfony 6.1 possibly:
symfony/symfony#43854

Removing the bundle before fix causes this error:
Screenshot from 2021-12-23 19-43-36



For now, we can add 2 lines to fetch entity explicitly. That way code is more clear and we can drup full package that is about to be deprecated... and not even supported by Symfony 6 👍

@samsonasik Could you verify locally this PR works well for you? I've tested demo, found an exception and fixed it this time. Just to be sure before merge :) Than you

@TomasVotruba TomasVotruba changed the title tv bump deps [WIP] Remove sensio if possible Dec 23, 2021
@TomasVotruba TomasVotruba changed the title [WIP] Remove sensio if possible Remove sensio if possible Dec 23, 2021
@samsonasik
Copy link
Member

I will try re-install docker first for it

@samsonasik
Copy link
Member

@TomasVotruba I tried locally with docker and open http://localhost:8080/demo and it seems not works:

  1. Open http://localhost:8080/demo
  2. click "Process" button
  3. display is still no change

Screen Shot 2021-12-24 at 13 11 49

  1. The URI is changed to:

http://localhost:8080/demo?rectorRun=1ec64802-dae0-6204-bbcd-c3f3e2892dd2

Looking at DB, it seems inserted, with the following data:

mysql> select * from rector_run \G
*************************** 1. row ***************************
                 id: ?H??b?????-?
        json_result: {"meta": {"config": null}, "totals": {"errors": 0, "changed_files": 1, "removed_node_count": 0, "removed_and_added_files_count": 0}, "file_diffs": [{"diff": "--- Original\n+++ New\n@@ -1,3 +1,3 @@\n <?php\r\n \r\n final class DemoFile\r\n@@ -5,8 +5,5 @@\n     public function run()\r\n     {\r\n         return 5;\r\n-\r\n-        // we never get here\r\n-        return 10;\r\n     }\r\n-}\n\\ No newline at end of file\n+}\n", "file": "../var/demo/vs0q2g4lfr6vfloptyd1/rector_analyzed_file.php", "applied_rectors": ["Rector\\DeadCode\\Rector\\FunctionLike\\RemoveCodeAfterReturnRector"], "applied_rectors_with_changelog": []}], "changed_files": ["../var/demo/vs0q2g4lfr6vfloptyd1/rector_analyzed_file.php"]}
fatal_error_message: NULL
             config: <?php

use Rector\Php74\Rector\Property\TypedPropertyRector;
use Rector\Set\ValueObject\SetList;
use Symfony\Component\DependencyInjection\Loader\Configurator\ContainerConfigurator;

return static function (ContainerConfigurator $containerConfigurator): void {
    $parameters = $containerConfigurator->parameters();
    $services = $containerConfigurator->services();

    // A. run whole set
    $containerConfigurator->import(SetList::DEAD_CODE);

    // B. or single rule
    $services->set(TypedPropertyRector::class);
};
            content: <?php

final class DemoFile
{
    public function run()
    {
        return 5;

        // we never get here
        return 10;
    }
}
         created_at: 2021-12-24 06:10:22
         updated_at: 2021-12-24 06:10:22
1 row in set (0.01 sec)

@samsonasik
Copy link
Member

Debugging the $jsonResult, it got the following output:

RectorRun.php on line 191:
array:4 [▼
  "meta" => array:1 [▼
    "config" => null
  ]
  "totals" => array:4 [▼
    "changed_files" => 1
    "removed_and_added_files_count" => 0
    "removed_node_count" => 0
    "errors" => 0
  ]
  "file_diffs" => array:1 [▼
    0 => array:4 [▼
      "file" => "../var/demo/e1ggff7qzyfqjn23hts4/rector_analyzed_file.php"
      "diff" => """
        --- Original


        +++ New


        @@ -1,3 +1,3 @@


         <?php


         


         final class DemoFile


        @@ -5,8 +5,5 @@


             public function run()


             {


                 return 5;


        -


        -        // we never get here


        -        return 10;


             }


        -}


        \ No newline at end of file


        +}


        """
      "applied_rectors" => array:1 [▶]
      "applied_rectors_with_changelog" => []
    ]
  ]
  "changed_files" => array:1 [▼
    0 => "../var/demo/e1ggff7qzyfqjn23hts4/rector_analyzed_file.php"
  ]
]

@samsonasik
Copy link
Member

@TomasVotruba it seems because it looking for $_GET for ?rectorRun value. It seems we can handle with:

$rectorRunUuid = $request->get('rectorRun');

@samsonasik
Copy link
Member

samsonasik commented Dec 24, 2021

@TomasVotruba Fixed 🎉 with check recturRun $_GET parameter value when exists, and verify if the value is valid 3da2ee9

@samsonasik
Copy link
Member

@TomasVotruba it works now with $_GET value check:

Screen Shot 2021-12-24 at 15 57 04

@samsonasik
Copy link
Member

All checks have passed 🎉 @TomasVotruba I think it is ready.

@samsonasik
Copy link
Member

samsonasik commented Dec 24, 2021

@TomasVotruba, I found the bug, it was because invalid route path definition:

         $demoDetailUrl = $this->urlGenerator->generate(RouteName::DEMO_DETAIL, [
-            'rectorRun' => $rectorRun->getId(),
+            'rectorRunUuid' => $rectorRun->getId(),
         ]);

as the route is defined as :

    #[Route(path: 'demo/{rectorRunUuid}', name: RouteName::DEMO_DETAIL, methods: ['GET'])]

Fixed at b9eeca3

@samsonasik
Copy link
Member

All checks have passed 🎉 @TomasVotruba I think it is ready. The generated route path redirect and display diff is now correct, generated as http://localhost:8080/demo/1ec649c7-f7c7-68ba-9652-41a3e6311d2e

Screen Shot 2021-12-24 at 16 33 27

@TomasVotruba
Copy link
Member Author

Thank you 👍 great job with the route redirect key!

@TomasVotruba TomasVotruba merged commit 9eb7f26 into main Dec 24, 2021
@TomasVotruba TomasVotruba deleted the tv-bump-deps branch December 24, 2021 10:28
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
Morty Proxy This is a proxified and sanitized view of the page, visit original site.