[DI] Change "this" to "that" in findAndSortTaggedServices doc to reduce confusion
#23578
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
The tag specified in the YAML spec is actually !!str.
… arg typehint (ogizanagi) This PR was merged into the 4.0-dev branch. Discussion ---------- [Translator] Uncomment YamlFileDumper constructor arg typehint | Q | A | ------------- | --- | Branch? | master <!-- see comment below --> | Bug fix? | no | New feature? | no <!-- don't forget updating src/**/CHANGELOG.md files --> | BC breaks? | no | Deprecations? | no <!-- don't forget updating UPGRADE-*.md files --> | Tests pass? | yes | Fixed tickets | N/A <!-- #-prefixed issue number(s), if any --> | License | MIT | Doc PR | N/A Commits ------- abe3440 [Translation] Uncomment YamlFileDumper constructor arg typehint
This PR was merged into the 4.0-dev branch. Discussion ---------- [Process] remove deprecated features | Q | A | ------------- | --- | Branch? | master | Bug fix? | no | New feature? | no | BC breaks? | yes | Deprecations? | no | Tests pass? | yes | Fixed tickets | | License | MIT | Doc PR | Commits ------- e4ec8e9 [Process] remove deprecated features
…e VoterInterface interface.
… --no-warmup option
…WybrenKoelmans)
This PR was merged into the 4.0-dev branch.
Discussion
----------
[Yaml][Lint] Add line numbers to JSON output.
| Q | A
| ------------- | ---
| Branch? | 2.7?
| Bug fix? | no?
| New feature? | yes?
| BC breaks? | no?
| Deprecations? | no?
| Tests pass? | Hopefully
| Fixed tickets | n/a
| License | MIT
| Doc PR | TODO?
- [ ] Run tests?
- [ ] Check if it will break BC?
- [ ] Update changelog?
The JSON output is not very useful for me without the line number. I don't want to have to parse it from the message.
Is this the right way of doing it?
With PR:
```
[
{
"file": "",
"line": 13,
"valid": false,
"message": "Unable to parse at line 13 (near \"sdf \")."
}
]
```
Before:
```
[
{
"file": "",
"valid": false,
"message": "Unable to parse at line 13 (near \"sdf \")."
}
]
```
Commits
-------
c6d19b1 [Yaml][Twig][Lint] Added line numbers to JSON output.
…hat don't implement VoterInterface. (hhamon) This PR was merged into the 4.0-dev branch. Discussion ---------- [Security] remove support for defining voters that don't implement VoterInterface. | Q | A | ------------- | --- | Branch? | master | Bug fix? | no | New feature? | no | BC breaks? | yes | Deprecations? | no | Tests pass? | yes | Fixed tickets | ~ | License | MIT | Doc PR | ~ Commits ------- f527790 [Security] remove support for defining voters that don't implement the VoterInterface interface.
…WybrenKoelmans)
This PR was merged into the 4.0-dev branch.
Discussion
----------
[Yaml][Lint] Add line numbers to JSON output.
| Q | A
| ------------- | ---
| Branch? | 2.7?
| Bug fix? | no?
| New feature? | yes?
| BC breaks? | no?
| Deprecations? | no?
| Tests pass? | Hopefully
| Fixed tickets | n/a
| License | MIT
| Doc PR | TODO?
- [ ] Run tests?
- [ ] Check if it will break BC?
- [ ] Update changelog?
The JSON output is not very useful for me without the line number. I don't want to have to parse it from the message.
Is this the right way of doing it?
With PR:
```
[
{
"file": "",
"line": 13,
"valid": false,
"message": "Unable to parse at line 13 (near \"sdf \")."
}
]
```
Before:
```
[
{
"file": "",
"valid": false,
"message": "Unable to parse at line 13 (near \"sdf \")."
}
]
```
Commits
-------
c6d19b1 [Yaml][Twig][Lint] Added line numbers to JSON output.
`getRealCurrentLineNb()` returns line numbers index by 0 (as they serve as array indexes internally).
…abbuh) This PR was merged into the 3.4 branch. Discussion ---------- [Yaml] fix the displayed line number | Q | A | ------------- | --- | Branch? | 3.4 | Bug fix? | yes | New feature? | no | BC breaks? | no | Deprecations? | no | Tests pass? | yes | Fixed tickets | symfony#23294 | License | MIT | Doc PR | First, this PR backports symfony#23294 to the `3.4` branch. Secondly, `getRealCurrentLineNb()` returns line numbers index by 0 (as they serve as array indexes internally). I removed the `getLastLineNumberBeforeDeprecation()` method added in symfony#23294 as we can just expose the already existing `getRealCurrentLineNb()` method. Commits ------- a2ae6bf [Yaml] fix the displayed line number 1baac5a feature symfony#23294 [Yaml][Lint] Add line numbers to JSON output. (WybrenKoelmans)
* 3.4: [Yaml] fix the displayed line number feature symfony#23294 [Yaml][Lint] Add line numbers to JSON output. (WybrenKoelmans)
…:clear without the --no-warmup option (ogizanagi) This PR was merged into the 3.3 branch. Discussion ---------- [FrameworkBundle] Display a proper warning on cache:clear without the --no-warmup option | Q | A | ------- 10000 ------ | --- | Branch? | 3.3 <!-- see comment below --> | Bug fix? | no | New feature? | no <!-- don't forget updating src/**/CHANGELOG.md files --> | BC breaks? | no | Deprecations? | no <!-- don't forget updating UPGRADE-*.md files --> | Tests pass? | yes | Fixed tickets | symfony/recipes#105 and others reports related to cache warming issues with final/custom kernel classes <!-- #-prefixed issue number(s), if any --> | License | MIT | Doc PR | N/A I'd suggest to simply make this more obvious for everyone, as the deprecation is only shown when executing the command with `-vv` or by inspecting logs otherwise: <img width="970" alt="screenshot 2017-06-29 a 20 08 21" src="https://user-images.githubusercontent.com/2211145/27703182-db734150-5d06-11e7-9306-f5837bf240ed.PNG"> Commits ------- 6cd188b [FrameworkBundle] Display a proper warning on cache:clear without the --no-warmup option
If one of the assertions failed, the clean up part would never happen.
This PR was merged into the 3.3 branch. Discussion ---------- [Dotenv] clean up before running assertions | Q | A | ------------- | --- | Branch? | 3.3 | Bug fix? | yes | New feature? | no | BC breaks? | no | Deprecations? | no | Tests pass? | yes | Fixed tickets | | License | MIT | Doc PR | If one of the assertions failed, the clean up part would never happen. Commits ------- baafc7b [Dotenv] clean up before running assertions
…description (xabbuh) This PR was merged into the 4.0-dev branch. Discussion ---------- [HttpFoundation] remove no longer valid docblock description | Q | A | ------------- | --- | Branch? | master | Bug fix? | yes | New feature? | no | BC breaks? | no | Deprecations? | no | Tests pass? | yes | Fixed tickets | symfony#22863 (comment) | License | MIT | Doc PR | Commits ------- 1beee41 remove no longer valid docblock description
…derers (xabbuh) This PR was merged into the 3.4 branch. Discussion ---------- [FrameworkBundle] disable unusable fragment renderers | Q | A | ------------- | --- | Branch? | 3.4 | Bug fix? | no | New feature? | yes | BC breaks? | no | Deprecations? | no | Tests pass? | yes | Fixed tickets | symfony#21733 | License | MIT | Doc PR | Commits ------- 2b3d7f0 disable unusable fragment renderers
…(chalasr) This PR was merged into the 3.3 branch. Discussion ---------- [Security] Fix Firewall ExceptionListener priority | Q | A | ------------- | --- | Branch? | 3.3 | Bug fix? | yes | New feature? | no | BC breaks? | no | Deprecations? | no | Tests pass? | yes | Fixed tickets | symfony#23253 | License | MIT | Doc PR | n/a When making EventDispatcher able to lazy load listeners, we stopped using `ContainerAwareEventDispatcher::addListenerService/addSubcriberService`, we use `EventDispatcher::addListener()` instead. This change makes that the order of listeners is different than before, because `ContainerAwareEventDispatcher` calls `addListener()` tardily so that factories are never stored in `EventDispatcher::$listeners`. Example diff due to the behavior change in 3.3 (registering an `AppBundle\ExceptionListener::doCatch()` exception listener in the fullstack): 3.2 ---- ```php array:5 0 => "Symfony\Component\Security\Http\Firewall\ExceptionListener::onKernelException" 1 => "AppBundle\ExceptionListener::doCatch" 2 => "Symfony\Component\HttpKernel\EventListener\ProfilerListener::onKernelException" 3 => "Symfony\Bundle\SwiftmailerBundle\EventListener\EmailSenderListener::onException" 4 => "Symfony\Component\HttpKernel\EventListener\ExceptionListener::onKernelException" ] ``` 3.3 ---- ```php array:5 [ 0 => "AppBundle\ExceptionListener::doCatch" 1 => "Symfony\Component\HttpKernel\EventListener\ProfilerListener::onKernelException" 2 => "Symfony\Bundle\SwiftmailerBundle\EventListener\EmailSenderListener::onException" 3 => "Symfony\Component\Security\Http\Firewall\ExceptionListener::onKernelException" 4 => "Symfony\Component\HttpKernel\EventListener\ExceptionListener::onKernelException" ] ``` (that is what breaks symfony#23253, the lazy listener is called before the runtime firewall exception listener on dispatch). This fixes the order by increasing the security exception listener priority. Commits ------- 8014b38 [Security] Fix Firewall ExceptionListener priority
This PR was merged into the 4.0-dev branch. Discussion ---------- Fix typo in docblock Commits ------- 6e88fef Fix typo in docblock
…INADDR_ANY) (jpauli, fabpot) This PR was merged into the 3.4 branch. Discussion ---------- [WebServer] Allow * to bind all interfaces (as INADDR_ANY) | Q | A | ------------- | --- | Branch? | 3.4 | Bug fix? | no | New feature? | yes | BC breaks? | no | Deprecations? | no | Tests pass? | no-tests | Fixed tickets | ~ | License | MIT | Doc PR | In Python and elsewhere, binding to '*' means '0.0.0.0' (INADDR_ANY). I just added that to WebServer command. Commits ------- 1880bcf fixed CS b31ebae Allow * to bind all interfaces (as INADDR_ANY)
…alues (xabbuh) This PR was merged into the 3.3 branch. Discussion ---------- [Dotenv] parse escaped quotes in unquoted env var values | Q | A | ------------- | --- | Branch? | 3.3 | Bug fix? | yes | New feature? | no | BC breaks? | no | Deprecations? | no | Tests pass? | yes | Fixed tickets | | License | MIT | Doc PR | Commits ------- 66a4fd7 parse escaped quotes in unquoted env var values
This PR was merged into the 3.4 branch. Discussion ---------- [Stopwatch] Add a reset method | Q | A | ------------- | --- | Branch? | 3.4 | Bug fix? | no | New feature? | yes | BC breaks? | no | Deprecations? | no | Tests pass? | yes | Fixed tickets | symfony#23284 | License | MIT | Doc PR | symfony/symfony-docs#8082 Let the Stopwatch to be reset to its original state, deleting all the data measured so far. This allows the stopwatch to be reusable, and emulates an actual stopwatch's reset button. Commits ------- 7cda099 [Stopwatch] Add a reset method
released v3.3.5
…grekas) This PR was merged into the 3.3 branch. Discussion ---------- [DI] Minor dumping logic simplification | Q | A | ------------- | --- | Branch? | 3.3 | Bug fix? | no | New feature? | no | BC breaks? | no | Deprecations? | no | Tests pass? | yes | Fixed tickets | - | License | MIT | Doc PR | - Likely not a bug fix because private services should be dealt with by some compiler pass, but anyway, locally, I don't get why non-shared private services should check the "$this->services" property. Did I miss something? Commits ------- 37d8495 [DI] Minor dumping logic simplification
* 3.2: [DI] Resolve aliases earlier [DI] Mark Container::$privates as internal bumped Symfony version to 3.2.13 updated VERSION for 3.2.12 updated CHANGELOG for 3.2.12 bumped Symfony version to 2.8.26 updated VERSION for 2.8.25 updated CHANGELOG for 2.8.25 bumped Symfony version to 2.7.33 updated VERSION for 2.7.32 update CONTRIBUTORS for 2.7.32 updated CHANGELOG for 2.7.32
* 3.3: [DI] Resolve aliases earlier [DI] Mark Container::$privates as internal [DI] Minor dumping logic simplification bumped Symfony version to 3.3.6 updated VERSION for 3.3.5 updated CHANGELOG for 3.3.5 bumped Symfony version to 3.2.13 updated VERSION for 3.2.12 updated CHANGELOG for 3.2.12 bumped Symfony version to 2.8.26 updated VERSION for 2.8.25 updated CHANGELOG for 2.8.25 bumped Symfony version to 2.7.33 updated VERSION for 2.7.32 update CONTRIBUTORS for 2.7.32 updated CHANGELOG for 2.7.32
* 3.4: [DI] Resolve aliases earlier [DI] Mark Container::$privates as internal [DI] Minor dumping logic simplification bumped Symfony version to 3.3.6 updated VERSION for 3.3.5 updated CHANGELOG for 3.3.5 bumped Symfony version to 3.2.13 updated VERSION for 3.2.12 updated CHANGELOG for 3.2.12 bumped Symfony version to 2.8.26 updated VERSION for 2.8.25 updated CHANGELOG for 2.8.25 bumped Symfony version to 2.7.33 updated VERSION for 2.7.32 update CONTRIBUTORS for 2.7.32 updated CHANGELOG for 2.7.32
This PR was merged into the 3.4 branch. Discussion ---------- [TwigBridge] fix tests | Q | A | ------------- | --- | Branch? | 3.4 | Bug fix? | yes | New feature? | no | BC breaks? | no | Deprecations? | no | Tests pass? | yes | Fixed tickets | | License | MIT | Doc PR | ~The changed test did not pass locally. I do probably miss something important, but I fail to see why this did not occur when running the tests on Travis CI.~ Never mind, I was looking at the wrong build. It fails on Travis CI too. Commits ------- 62410b6 [TwigBridge] fix tests
* 3.4: [TwigBridge] fix tests
This PR was merged into the 4.0-dev branch. Discussion ---------- [TwigBridge] remove deprecated features | Q | A | ------------- | --- | Branch? | master | Bug fix? | no | New feature? | no | BC breaks? | yes | Deprecations? | no | Tests pass? | yes | Fixed tickets | | License | MIT | Doc PR | Commits ------- aae494c [TwigBridge] remove deprecated features
* 3.3: remove useless comment fix merge
…ent ServiceSubscriberInterface (nicolas-grekas) This PR was merged into the 3.4 branch. Discussion ---------- [FrameworkBundle] Make RouterCacheWarmer implement ServiceSubscriberInterface | Q | A | ------------- | --- | Branch? | 3.4 | Bug fix? | no | New feature? | yes | BC breaks? | no | Deprecations? | yes | Tests pass? | yes | Fixed tickets | - | License | MIT | Doc PR | - Blocks symfony#23569 Commits ------- c0af003 [FrameworkBundle] Make RouterCacheWarmer implement ServiceSubscriberInterface
* 3.4: remove useless comment [FrameworkBundle] Make RouterCacheWarmer implement ServiceSubscriberInterface fix merge
I know this is extremely minor, but reading the description of this method, I got confused. Where it says: > The order of additions must be respected for services having the same priority, and knowing that the \SplPriorityQueue class does not respect the FIFO method, we should not use *this* class. Should it not say "we should not use *that* class"?
findAndSortTaggedServices doc to reduce confusionfindAndSortTaggedServices doc to reduce confusion
|
that's a good way to do a first contrib :) |
|
Oh wow. GitHub did not do what I expected. Going to close this and open a new PR. Sorry anybody that just got pinged! |
fabpot
added a commit
that referenced
this pull request
Jul 19, 2017
…es` doc to reduce confusion (simshaun) This PR was merged into the 3.2 branch. Discussion ---------- [DI] Change "this" to "that" in `findAndSortTaggedServices` doc to reduce confusion Continuation of PR #23578 which I royally messed up..... | Q | A | ------------- | --- | Branch? | 3.2 | Bug fix? | no | New feature? | no | BC breaks? | no | Deprecations? | no | Tests pass? | yes | License | MIT | Doc PR | None I know this is extremely minor, but reading the description of this method, I got confused. Wondering if it's just me. Where it says: > The order of additions must be respected for services having the same priority, and knowing that the \SplPriorityQueue class does not respect the FIFO method, we should not use **this** class. Should it not say "we should not use **that** class"? Commits ------- 04b7b04 Change "this" to "that" to avoid confusion
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
I know this is extremely minor, but reading the description of this method, I got confused. Wondering if it's just me.
Where it says:
Should it not say "we should not use that class"?