Skip to content

Remove unused private methods #49517

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

Conversation

alamirault
Copy link
Contributor

Q A
Branch? 6.3
Bug fix? no
New feature? no
Deprecations? no
Tickets Fix #...
License MIT
Doc PR symfony/symfony-docs#...

I searched all unused private methods in codebase and found these

@carsonbot carsonbot added this to the 6.3 milestone Feb 23, 2023
@@ -854,25 +854,6 @@ public function testCreateViewFlatlabelTranslationParametersClosureReceivesValue
$this->assertFlatViewWithlabelTranslationParameters($view);
}

private function assertScalarListWithChoiceValues(ChoiceListInterface $list)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Copy link
Member

@nicolas-grekas nicolas-grekas left a comment

Choose a reason for hiding this comment

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

LGTM except the trait

@alamirault alamirault requested review from nicolas-grekas and removed request for xabbuh and yceruto February 23, 2023 20:51
@nicolas-grekas nicolas-grekas force-pushed the feature/remove-unused-private-methods branch from 04525d3 to 53322dc Compare February 23, 2023 20:52
@nicolas-grekas
Copy link
Member

Thank you @alamirault.

@nicolas-grekas nicolas-grekas merged commit 005faa6 into symfony:6.3 Feb 23, 2023
@alamirault alamirault deleted the feature/remove-unused-private-methods branch February 23, 2023 20:55
nicolas-grekas added a commit that referenced this pull request Sep 20, 2023
…w route to a collection (bram123)

This PR was squashed before being merged into the 6.3 branch.

Discussion
----------

[Routing] Fix routing collection defaults when adding a new route to a collection

| Q             | A
| ------------- | ---
| Branch?       | 6.3
| Bug fix?      | yes
| New feature?  | no
| Deprecations? | no
| Tickets       | ~
| License       | MIT
| Doc PR        | ~

In our current Symfony project we use PHP routing files, and use routing collections that are setup before adding routes.

Before upgrading to Symfony 6.3 this meant we could add an attribute to the collection, add X routes and they would all have this routing attribute (be it stateless / methods / defaults). After upgrading to S6.3 this no longer works.

PR #49517 removed the createRoute method from the CollectionConfigurator, because it was thought to be unused, but this method was used when adding a new Route to a RouteCollection. Without the method, the createRoute method of the LocalizedRouteTrait was used.
This means that when setting up a route collection before adding routes, the new routes no longer get the collection settings.

Added a new unittest to prevent this from happening again. For now only added to the PhpFileLoaderTest, I don't know if this is also possible for XML/YML routing config to also add a test in those LoaderTests.

Commits
-------

bfdd16e [Routing] Fix routing collection defaults when adding a new route to a collection
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.

3 participants