Skip to content

Navigation Menu

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

[Routing] Fix name-prefixing when using PHP DSL #26597

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 1 commit into from
Mar 19, 2018

Conversation

nicolas-grekas
Copy link
Member

Q A
Branch? master
Bug fix? yes
New feature? no
BC breaks? no
Deprecations? no
Tests pass? yes
Fixed tickets -
License MIT
Doc PR -

Fixes bad implem merged in #25178

@nicolas-grekas nicolas-grekas added this to the 4.1 milestone Mar 19, 2018
@nicolas-grekas nicolas-grekas merged commit 0053eee into symfony:master Mar 19, 2018
nicolas-grekas added a commit that referenced this pull request Mar 19, 2018
…rekas)

This PR was merged into the 4.1-dev branch.

Discussion
----------

[Routing] Fix name-prefixing when using PHP DSL

| Q             | A
| ------------- | ---
| Branch?       | master
| Bug fix?      | yes
| New feature?  | no
| BC breaks?    | no
| Deprecations? | no
| Tests pass?   | yes
| Fixed tickets | -
| License       | MIT
| Doc PR        | -

Fixes bad implem merged in #25178

Commits
-------

0053eee [Routing] Fix name-prefixing when using PHP DSL
@nicolas-grekas nicolas-grekas deleted the route-name-prefixfix branch March 19, 2018 11:55
@@ -40,8 +40,14 @@ public function __destruct()
*
* @return $this
*/
final public function prefix($prefix)
final public function prefix($prefix, string $namePrefix = '')
Copy link
Contributor

Choose a reason for hiding this comment

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

IMO addng a name prefix should be a separate method again. Those are totally unrelated concepts and I can add a name prefix without having to add a path prefix. This is also possible in the annotation loader for example. So it not consistent to do do it in one method.

Copy link
Member Author

Choose a reason for hiding this comment

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

agreed, done in #26284

fabpot added a commit that referenced this pull request May 13, 2018
This PR was squashed before being merged into the 4.1 branch (closes #27202).

Discussion
----------

[Messenger] Improve the profiler panel

| Q             | A
| ------------- | ---
| Branch?       | 4.1 <!-- see below -->
| Bug fix?      | no
| New feature?  | yes <!-- don't forget to update src/**/CHANGELOG.md files -->
| BC breaks?    | no     <!-- see https://symfony.com/bc -->
| Deprecations? | no <!-- don't forget to update UPGRADE-*.md and src/**/CHANGELOG.md files -->
| Tests pass?   | yes <!-- please add some, will be required by reviewers -->
| Fixed tickets | N/A   <!-- #26597  issue number(s), if any -->
| License       | MIT
| Doc PR        | N/A

This is an attempt to enhance the profiler panel a bit.

**with the following messages dispatched:**

```php
$queryBus->dispatch(Envelope::wrap(new GetGreetingsQuery('Hello you'))
    ->with(new JustAFriendOfMine())
    ->with(new AndHisPlusO̶n̶e̶Eleven())
);
$commandBus->dispatch(new SendGiftCommand());
$queryBus->dispatch(new GetGreetingsQuery('Exterminate!'));
```

## Before

<img width="1084" alt="screenshot 2018-05-12 a 13 57 57" src="https://user-images.githubusercontent.com/2211145/39957055-8a0f009e-55ec-11e8-9d8e-bf79aad4b420.PNG">

🐛calls order are wrong here, fixed in this PR

## After

### collapsed

<!-- <img width="1083" alt="screenshot 2018-05-10 a 23 51 07" src="https://user-images.githubusercontent.com/2211145/39896093-19a8c7ee-54ad-11e8-8dcb-4e165ffd2eae.PNG">-->

<img width="1085" alt="screenshot 2018-05-12 a 13 18 35" src="https://user-images.githubusercontent.com/2211145/39956802-9d4c38a2-55e7-11e8-8425-ad090c0871b6.PNG">
<img width="1085" alt="screenshot 2018-05-12 a 13 26 44" src="https://user-images.githubusercontent.com/2211145/39956827-25d9e426-55e8-11e8-9116-160603649f33.PNG">

📝 _When loading the page, all messages details are collapsed by default but the first one per tab._

### expanded

<!-- <img width="1083" alt="screenshot 2018-05-10 a 23 13 39" src="https://user-images.githubusercontent.com/2211145/39894779-42c9cc9a-54a8-11e8-9529-6292481536d4.PNG"> -->

<img width="1086" alt="screenshot 2018-05-12 a 13 49 42" src="https://user-images.githubusercontent.com/2211145/39956981-639fc3d6-55eb-11e8-9224-a48f591db3da.PNG">

### live

<!-- ![mai-10-2018 23-16-17](https://user-images.githubusercontent.com/2211145/39894789-4b8fa138-54a8-11e8-986c-fccf6cd0234f.gif) -->

![mai-12-2018 13-55-40](https://user-images.githubusercontent.com/2211145/39957041-37f17b34-55ec-11e8-8569-a733a104bf82.gif)

### toolbar (with exceptions)

<img width="284" alt="screenshot 2018-05-10 a 23 18 32" src="https://user-images.githubusercontent.com/2211145/39895011-0467f2a0-54a9-11e8-9d78-25461cf71c41.PNG">

## Notes

- Table headers are clickable, so you can jump directly to the message class in the code
- Reversing headers/rows allows to have a wider space for dumps and allows to add more entries in the future. This is an issue we already have with the Validator panel (when there is both an invalid value as object and a constraint violation dumped) which I'd like to revamp soon.
- ~~I wonder if we should keep the dispatched messages in call order, or if we can segregate by bus (using tabs?).~~
- ~~we could add a left container listing messages classes only, allowing to show details of a single message dispatched on a right container (similar to what the Form panel does). I'll probably suggest the same for the Validator panel.~~

Commits
-------

3d19578 [Messenger] Improve the profiler panel
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.