Skip to content

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

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

Conversation

noemi-salaun
Copy link
Contributor

@noemi-salaun 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 feature/add-debug-deprecations-command branch 2 times, most recently from 7d99b07 to 090219a Compare March 7, 2020 12:16
@chalasr chalasr added this to the next milestone Mar 7, 2020
@chalasr
Copy link
Member

chalasr commented Mar 7, 2020

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

@noemi-salaun
Copy link
Contributor Author

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 feature/add-debug-deprecations-command branch from 090219a to bb483d1 Compare March 7, 2020 12:47
@noemi-salaun noemi-salaun changed the base branch from 4.4 to master March 7, 2020 12:47
@noemi-salaun
Copy link
Contributor Author

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

@maxhelias
Copy link
Contributor

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

@noemi-salaun
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 feature/add-debug-deprecations-command branch from bb483d1 to ea53d42 Compare March 7, 2020 17:02
Copy link
Contributor

@noniagriconomie noniagriconomie left a comment

Choose a reason for hiding this comment

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

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 feature/add-debug-deprecations-command branch from ea53d42 to 8215063 Compare March 7, 2020 17:30
Copy link
Contributor

@ogizanagi ogizanagi left a comment

Choose a reason for hiding this comment

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

Thank you for taking over this

Copy link
Contributor

@maxhelias maxhelias left a comment

Choose a reason for hiding this comment

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

Great! it's look good to me

@noemi-salaun noemi-salaun force-pushed the feature/add-debug-deprecations-command branch 2 times, most recently from 022653f to 44d85b6 Compare March 9, 2020 17:57
@noemi-salaun
Copy link
Contributor Author

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 March 10, 2020 19:20
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 feature/add-debug-deprecations-command branch from 44d85b6 to 6e2e068 Compare March 12, 2020 17:28
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.

Thanks @noemi-salaun for taking over!

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

public function testGetDeprecationNoFile(): void
Copy link
Member

Choose a reason for hiding this comment

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

void should be removed

@nicolas-grekas nicolas-grekas force-pushed the feature/add-debug-deprecations-command branch from 6e2e068 to ee6391e Compare March 13, 2020 12:34
@nicolas-grekas
Copy link
Member

Thank you @noemi-salaun.

@nicolas-grekas
Copy link
Member

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
Development

Successfully merging this pull request may close these issues.

9 participants