Skip to content
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

[FrameworkBundle] add --deprecations on debug:container command #35995

Conversation

@noemi-salaun
Copy link
Contributor

noemi-salaun commented Mar 7, 2020

Q A
Branch? master (5.1)
Bug fix? no
New feature? yes
Deprecations? no
Tickets #30089
License MIT
Doc PR todo

I apologize in advance, I am not fluent in English

Continuity of @Simperfit work from his PR #32584

I added support for XML, JSON and markdown formats as well as unit tests for all formats.

XML format

<?xml version="1.0" encoding="UTF-8"?>
<deprecations remainingCount="5">
  <deprecation count="3">
    <message>Some deprecation message.</message>
    <file>/path/to/some/file.php</file>
    <line>39</line>
  </deprecation>
  <deprecation count="2">
    <message>An other deprecation message.</message>
    <file>/path/to/an/other/file.php</file>
    <line>25</line>
  </deprecation>
</deprecations>

JSON format

{
  "remainingCount": 5,
  "deprecations": [
    {
      "message": "Some deprecation message.",
      "file": "\/path\/to\/some\/file.php",
      "line": 39,
      "count": 3
    },
    {
      "message": "An other deprecation message.",
      "file": "\/path\/to\/an\/other\/file.php",
      "line": 25,
      "count": 2
    }
  ]
}

Markdown format

Remaining deprecations (5)

  • 3x: "Some deprecation message." in /path/to/some/file.php:39
  • 2x: "An other deprecation message." in /path/to/an/other/file.php:25

I added a new commit on top of @Simperfit 's one, but I don't know how you would like to have the git history writed.

@noemi-salaun noemi-salaun force-pushed the noemi-salaun:feature/add-debug-deprecations-command branch 2 times, most recently from 7d99b07 to 090219a Mar 7, 2020
@chalasr chalasr added this to the next milestone Mar 7, 2020
@chalasr

This comment has been minimized.

Copy link
Member

chalasr commented Mar 7, 2020

Thanks for the taking over!
Features are for the master branch :) Rebase needed

@noemi-salaun

This comment has been minimized.

Copy link
Contributor Author

noemi-salaun commented Mar 7, 2020

Oh ok, I was targeting the same version as the previous PR but I guess 5.0 was release since ^^

@noemi-salaun noemi-salaun force-pushed the noemi-salaun:feature/add-debug-deprecations-command branch from 090219a to bb483d1 Mar 7, 2020
@noemi-salaun noemi-salaun changed the base branch from 4.4 to master Mar 7, 2020
@noemi-salaun

This comment has been minimized.

Copy link
Contributor Author

noemi-salaun commented Mar 7, 2020

Oh I rebased before switching the targeted branch for this PR, so the CI failed

@maxhelias

This comment has been minimized.

Copy link
Contributor

maxhelias commented Mar 7, 2020

The history of commits is weird, can you take a look for remove the commit alreay merge

@noemi-salaun

This comment has been minimized.

Copy link
Contributor Author

noemi-salaun commented Mar 7, 2020

I have restarted from Simperfit branch, rebase it on top of master and then cherry pick my commit. I think it should be good now :)

@noemi-salaun noemi-salaun force-pushed the noemi-salaun:feature/add-debug-deprecations-command branch from bb483d1 to ea53d42 Mar 7, 2020
Copy link
Contributor

noniagriconomie left a comment

Wanted to give it a try also, you beat me at this
Thank you continuing this, i think it will be very usefull

@noemi-salaun noemi-salaun force-pushed the noemi-salaun:feature/add-debug-deprecations-command branch from ea53d42 to 8215063 Mar 7, 2020
Copy link
Member

ogizanagi left a comment

Thank you for taking over this

Copy link
Contributor

maxhelias left a comment

Great! it's look good to me

@noemi-salaun noemi-salaun force-pushed the noemi-salaun:feature/add-debug-deprecations-command branch 2 times, most recently from 022653f to 44d85b6 Mar 9, 2020
@SelviA
SelviA approved these changes Mar 10, 2020
@noemi-salaun

This comment has been minimized.

Copy link
Contributor Author

noemi-salaun commented Mar 10, 2020

I think @dunglas, @jderusse, @lyrixx, @sroze and @xabbuh were automatically added as reviewer when I strangely rebase between branch 4.4 and master. The PR contained their commit by mistake. But I cannot remove them from the reviewers... I'm sorry for the flood 😞

@ogizanagi ogizanagi requested review from chalasr and removed request for dunglas, lyrixx, jderusse, sroze and xabbuh Mar 10, 2020
nicolas-grekas added a commit that referenced this pull request Mar 12, 2020
…criptor (noemi-salaun)

This PR was merged into the 3.4 branch.

Discussion
----------

[FrameworkBundle] remove redundant PHPDoc in console Descriptor

| Q             | A
| ------------- | ---
| Branch?       | 3.4
| Bug fix?      | no
| New feature?  | no <!-- please update src/**/CHANGELOG.md files -->
| Deprecations? | no <!-- please update UPGRADE-*.md and src/**/CHANGELOG.md files -->
| Tickets       | None <!-- prefix each issue number with "Fix #", if any -->
| License       | MIT
| Doc PR        | N/A

The PHPDoc for some `describeXXX` methods in the abstract `Symfony\Bundle\FrameworkBundle\Console\Descriptor\Descriptor` was inaccurate or redundant.

I remove the PHPDoc in the superclass and the `{@inheritdoc}` in the child class as suggested by @chalasr in this PR comment #35995 (comment)

I keep the PHPDoc when it adds some explanation about the method or its params.

For exemple :
### inaccurate:
```php
    /**
     * Describes an InputArgument instance.
     */
    abstract protected function describeRouteCollection(RouteCollection $routes, array $options = []);

    /**
     * Describes an InputOption instance.
     */
    abstract protected function describeRoute(Route $route, array $options = []);
```

### redundant
```php
    /**
     * Describes container parameters.
     */
    abstract protected function describeContainerParameters(ParameterBag $parameters, array $options = []);

    /**
     * Describes container tags.
     */
    abstract protected function describeContainerTags(ContainerBuilder $builder, array $options = []);
```

### kept

```php
    /**
     * Describes event dispatcher listeners.
     *
     * Common options are:
     * * name: name of listened event
     */
    abstract protected function describeEventDispatcherListeners(EventDispatcherInterface $eventDispatcher, array $options = []);

    /**
     * Describes a callable.
     *
     * @param mixed $callable
     */
    abstract protected function describeCallable($callable, array $options = []);
```
<!--
Replace this notice by a short README for your feature/bugfix. This will help people
understand your PR and can be used as a start for the documentation.

Additionally (see https://symfony.com/releases):
 - Always add tests and ensure they pass.
 - Never break backward compatibility (see https://symfony.com/bc).
 - Bug fixes must be submitted against the lowest maintained branch where they apply
   (lowest branches are regularly merged to upper ones so they get the fixes too.)
 - Features and deprecations must be submitted against branch master.
-->

Commits
-------

e535e7d [FrameworkBundle] remove redundant PHPDoc in console Descriptor and subclass
@noemi-salaun noemi-salaun force-pushed the noemi-salaun:feature/add-debug-deprecations-command branch from 44d85b6 to 6e2e068 Mar 12, 2020
Copy link
Member

nicolas-grekas left a comment

Thanks @noemi-salaun for taking over!

$this->assertStringContainsString('[OK] There are no deprecations in the logs!', $tester->getDisplay());
}

public function testGetDeprecationNoFile(): void

This comment has been minimized.

Copy link
@nicolas-grekas

nicolas-grekas Mar 13, 2020

Member

void should be removed

@nicolas-grekas nicolas-grekas force-pushed the noemi-salaun:feature/add-debug-deprecations-command branch from 6e2e068 to ee6391e Mar 13, 2020
@nicolas-grekas

This comment has been minimized.

Copy link
Member

nicolas-grekas commented Mar 13, 2020

Thank you @noemi-salaun.

@nicolas-grekas nicolas-grekas merged commit d028a50 into symfony:master Mar 13, 2020
1 of 3 checks passed
1 of 3 checks passed
continuous-integration/appveyor/pr Waiting for AppVeyor build to complete
Details
continuous-integration/travis-ci/pr The Travis CI build is in progress
Details
fabbot.io Your code looks good.
Details
@nicolas-grekas

This comment has been minimized.

Copy link
Member

nicolas-grekas commented Mar 13, 2020

And thank you @Simperfit of course also!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

9 participants
You can’t perform that action at this time.