Movatterモバイル変換


[0]ホーム

URL:


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

[Routing] Deprecate RouteCollectionBuilder#32937

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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Conversation

vudaltsov
Copy link
Contributor

@vudaltsovvudaltsov commentedAug 5, 2019
edited
Loading

QA
Branch?5.1
Bug fix?no
New feature?yes
BC breaks?no
Deprecations?yes
Tests pass?yes
Fixed tickets#32240
LicenseMIT
Doc PRsymfony/symfony-docs#12688
Recipe PRsymfony/recipes#690

A lot to be done here after the implementation is accepted:

  • finish deprecations in the MicroKernelTrait
  • deprecate the class
  • mention in the CHANGELOG file
  • mention in the UPGRADE file
  • mark tests as legacy
  • add a doc PR
  • update the recipe

Ping@Tobion ,@nicolas-grekas .

TomasVotruba reacted with thumbs up emoji
@nicolas-grekasnicolas-grekas added this to thenext milestoneAug 5, 2019
@vudaltsovvudaltsov changed the title[WIP] Deprecate RouteCollectionBuilder[Routing][WIP] Deprecate RouteCollectionBuilderAug 5, 2019
@vudaltsov
Copy link
ContributorAuthor

@nicolas-grekas ,@Tobion , could you please give me some feedback so that I can move on?

@nicolas-grekas
Copy link
Member

@Tobion would you have time to help move this one forward soonish? Otherwise, I'd suggest to close (and keep the issue open)

@Tobion
Copy link
Contributor

I won't be able to work on this as other issues have more priority to me like#33065 and#32077

@vudaltsov
Copy link
ContributorAuthor

@nicolas-grekas , don't close the PR, please. I will have time to make it ready for review soon. At least we'll discuss it at the hack day in Lisbon ;)

@vudaltsovvudaltsovforce-pushed the32240-deprecate-route-collection-builder branch from570a239 to16d5a6dCompareNovember 10, 2019 16:24
@vudaltsovvudaltsov changed the base branch from4.4 tomasterNovember 10, 2019 16:24
@vudaltsovvudaltsov marked this pull request as ready for reviewNovember 10, 2019 16:26
@vudaltsovvudaltsov changed the title[Routing][WIP] Deprecate RouteCollectionBuilder[Routing] Deprecate RouteCollectionBuilderNov 10, 2019
@vudaltsov
Copy link
ContributorAuthor

@nicolas-grekas , ready for review with tests and deprecations!

I will finish the CHANGELOG and add the docs once you agree with the implementation.

For now I rebased onto master. Do we have any chance to get this in 4.4? Or it's too late?

@vudaltsov
Copy link
ContributorAuthor

@nicolas-grekas , fully ready for you review

Copy link
Member

@nicolas-grekasnicolas-grekas left a comment

Choose a reason for hiding this comment

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

I really like the idea! Would be great to do the same for services late on ;)

vudaltsov and TomasVotruba reacted with thumbs up emoji
@vudaltsov
Copy link
ContributorAuthor

vudaltsov commentedNov 24, 2019
edited
Loading

Yes, I already started in the airplane yesterday ;)

rvanlaak and TomasVotruba reacted with hooray emoji

@vudaltsov
Copy link
ContributorAuthor

@fabpot, thanx, corrected!

@fabpotfabpotforce-pushed the32240-deprecate-route-collection-builder branch frombadef1d toe641cbdCompareNovember 25, 2019 12:06
@fabpot
Copy link
Member

Thank you@vudaltsov.

OskarStark and TomasVotruba reacted with heart emoji

@fabpotfabpot merged commite641cbd intosymfony:masterNov 25, 2019
@vudaltsovvudaltsov deleted the 32240-deprecate-route-collection-builder branchNovember 25, 2019 12:07
nicolas-grekas added a commit that referenced this pull requestDec 13, 2019
…in MicroKernelTrait::configureContainer() (nicolas-grekas)This PR was merged into the 5.1-dev branch.Discussion----------[FrameworkBundle] Allow using a ContainerConfigurator in MicroKernelTrait::configureContainer()| Q             | A| ------------- | ---| Branch?       | master| Bug fix?      | no| New feature?  | yes| Deprecations? | no| Tickets       | -| License       | MIT| Doc PR        | -This PR is a continuation of#32937 (it reverts some parts of it that are not needed anymore). It builds on#34872 for now.This PR allows using the PHP-DSL natively in our `Kernel::configureContainer()` methods.It allows the same in our `Kernel::configureRoutes()` methods.Both signatures are handled gracefully with no deprecations to let existing code in peace:- `protected function configureContainer(ContainerBuilder $c, LoaderInterface $loader);`- `protected function configureContainer(ContainerConfigurator $c);` (a loader is always passed as 2nd arg to ease FC)Same for routes:- `protected function configureRoutes(RoutingConfigurator $routes);`- `protected function configureRoutes(RouteCollectionBuilder $routes);` (this one is deprecated because `RouteCollectionBuilder` is deprecated)Here is my working `src/Kernel.php` after this change:```phpclass Kernel extends BaseKernel{    use MicroKernelTrait;    protected function configureContainer(ContainerConfigurator $container): void    {        $container->import('../config/{packages}/*.yaml');        $container->import('../config/{packages}/'.$this->environment.'/*.yaml');        $container->import('../config/services.yaml');        $container->import('../config/{services}_'.$this->environment.'.yaml');    }    protected function configureRoutes(RoutingConfigurator $routes): void    {        $routes->import('../config/{routes}/'.$this->environment.'/*.yaml');        $routes->import('../config/{routes}/*.yaml');        $routes->import('../config/routes.yaml');    }}```Commits-------cf45eec [FrameworkBundle] Allow using a ContainerConfigurator in MicroKernelTrait::configureContainer()
@nicolas-grekasnicolas-grekas modified the milestones:next,5.1May 4, 2020
@fabpotfabpot mentioned this pull requestMay 5, 2020
Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment
Reviewers

@fabpotfabpotfabpot approved these changes

@nicolas-grekasnicolas-grekasnicolas-grekas approved these changes

Assignees
No one assigned
Projects
None yet
Milestone
5.1
Development

Successfully merging this pull request may close these issues.

5 participants
@vudaltsov@nicolas-grekas@Tobion@fabpot@carsonbot

[8]ページ先頭

©2009-2025 Movatter.jp