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

Sign up
Appearance settings

Drop useless call to end method #1359

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
makasim merged 1 commit into php-enqueue:master from ddziaduch:patch-1
Sep 24, 2024
Merged

Drop useless call to end method #1359

makasim merged 1 commit into php-enqueue:master from ddziaduch:patch-1
Sep 24, 2024

Conversation

Copy link
Contributor

@ddziaduch ddziaduch commented Sep 17, 2024

Due to this, upgrading from SF 6.4.7 to more recent versions is impossible.

Output from the cache:clear after upgrade:

Executing script cache:clear [KO]
 [KO]
Script cache:clear returned with error code 255
!! TypeError {#5946
!! #message: "Symfony\Component\Config\Definition\Builder\NodeDefinition::end(): Return value must be of type Symfony\Component\Config\Definition\Builder\NodeParentInterface, null returned"
!! #code: 0
!! #file: "./vendor/symfony/config/Definition/Builder/NodeDefinition.php"
!! #line: 98
!! trace: {
!! ./vendor/symfony/config/Definition/Builder/NodeDefinition.php:98 { ...}
!! ./vendor/enqueue/enqueue/Symfony/Client/DependencyInjection/ClientFactory.php:82 { ...}
!! ./vendor/enqueue/enqueue-bundle/DependencyInjection/Configuration.php:42 { ...}
!! ./vendor/symfony/config/Definition/Processor.php:46 { ...}
!! ./vendor/symfony/dependency-injection/Extension/Extension.php:109 { ...}
!! ./vendor/enqueue/enqueue-bundle/DependencyInjection/EnqueueExtension.php:34 { ...}
!! ./vendor/symfony/dependency-injection/Compiler/MergeExtensionConfigurationPass.php:76 { ...}
!! ./vendor/symfony/http-kernel/DependencyInjection/MergeExtensionConfigurationPass.php:42 { ...}
!! ./vendor/symfony/dependency-injection/Compiler/Compiler.php:80 { ...}
!! ./vendor/symfony/dependency-injection/ContainerBuilder.php:767 { ...}
!! ./vendor/symfony/http-kernel/Kernel.php:506 { ...}
!! ./vendor/symfony/http-kernel/Kernel.php:771 { ...}
!! ./vendor/symfony/http-kernel/Kernel.php:126 { ...}
!! ./vendor/symfony/framework-bundle/Console/Application.php:190 { ...}
!! ./vendor/symfony/framework-bundle/Console/Application.php:72 { ...}
!! ./vendor/symfony/console/Application.php:175 { ...}
!! ./bin/console:43 {
!! › $application = new Application($kernel);
!! › $application->run($input);
!! › 
!! }
!! }
!! }
!! 2024年09月17日T12:45:24+00:00 [critical] Uncaught Error: Symfony\Component\Config\Definition\Builder\NodeDefinition::end(): Return value must be of type Symfony\Component\Config\Definition\Builder\NodeParentInterface, null returned
!! 
Script @auto-scripts was called via post-install-cmd

Due to this, upgrading from SF 6.4.7 to more recent versions is impossible.
Copy link
Contributor Author

@makasim @p-pichet can this be merged? It is quite important for us

Copy link
Member

makasim commented Sep 19, 2024

sorry, I might be missing something. What has been changed, except for indentions?

Copy link
Contributor Author

@makasim at the end of the method there were three calls to end() method, now there are just two - the last was unnecessary

Copy link
Contributor

@Steveb-p Steveb-p left a comment

Choose a reason for hiding this comment

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

The change is correct.

We accidentally went and called Node::end() one additional time. Because there is no node above (which is what end method gives) it started failing once this method became strong-typed in Symfony.

In theory there is no need to call end() method at all, if we're not interested in traversing the tree. But it is done across all Symfony examples and is otherwise safe to do.

Copy link
Contributor

Base on the explanation, it's ok for me, but can you add indentation please ?
I don't have right for merge it

Copy link
Member

makasim commented Sep 24, 2024

got it, thanks!

@makasim makasim merged commit 3c1c778 into php-enqueue:master Sep 24, 2024
34 checks passed
@ddziaduch ddziaduch deleted the patch-1 branch September 24, 2024 07:58
Copy link
Contributor Author

@makasim can you please create a new tag? Thanks in advance 🙂

Copy link
Contributor Author

Is anybody able to help with this tag? @makasim @p-pichet @Steveb-p maybe?

Copy link
Member

makasim commented Oct 1, 2024

p-pichet reacted with thumbs up emoji ddziaduch and Veridis reacted with heart emoji

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Steveb-p Steveb-p Steveb-p approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

AltStyle によって変換されたページ (->オリジナル) /