Skip to content

removed useless PHPDoc #24342

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

Closed
wants to merge 4 commits into from
Closed

removed useless PHPDoc #24342

wants to merge 4 commits into from

Conversation

OskarStark
Copy link
Contributor

@OskarStark OskarStark commented Sep 27, 2017

Q A
Branch? 2.7
Bug fix? no
New feature? no
BC breaks? no
Deprecations? no
Tests pass? yes
Fixed tickets
License MIT
Doc PR n/a

@OskarStark
Copy link
Contributor Author

@lyrixx
Copy link
Member

lyrixx commented Sep 27, 2017

Yes please

@OskarStark OskarStark changed the title removed useless PHPDoc description removed useless PHPDoc Sep 27, 2017
@OskarStark OskarStark changed the title removed useless PHPDoc [WIP] removed useless PHPDoc Sep 27, 2017
@ogizanagi
Copy link
Contributor

It should start by targeting the lowest maintained branch (i.e 2.7), not 3.3. :)

@ogizanagi ogizanagi added this to the 2.7 milestone Sep 27, 2017
@OskarStark OskarStark changed the title [WIP] removed useless PHPDoc removed useless PHPDoc Sep 27, 2017
@OskarStark
Copy link
Contributor Author

It should start by targeting the lowest maintained branch (i.e 2.7), not 3.3. :)

can you do this or shall I switch the branch?

@Simperfit
Copy link
Contributor

You should do it since it will lead to a rebase

@OskarStark OskarStark changed the base branch from 3.3 to 2.7 September 28, 2017 06:36
@OskarStark OskarStark changed the base branch from 2.7 to 3.3 September 28, 2017 06:36
@OskarStark OskarStark changed the base branch from 3.3 to 2.7 September 28, 2017 06:42
@OskarStark OskarStark changed the title removed useless PHPDoc [WIP] removed useless PHPDoc Sep 28, 2017
@OskarStark
Copy link
Contributor Author

Refs PHP-CS-Fixer/PHP-CS-Fixer#2511

@OskarStark OskarStark changed the title [WIP] removed useless PHPDoc removed useless PHPDoc Sep 28, 2017
@OskarStark
Copy link
Contributor Author

It should start by targeting the lowest maintained branch (i.e 2.7), not 3.3. :)

done @ogizanagi

@ogizanagi
Copy link
Contributor

@OskarStark: There are still some tests failing.

@OskarStark
Copy link
Contributor Author

@OskarStark: There are still some tests failing.

fabbot is failing, but not related to this PR
AppVeyor is green
travis failed, due to now output for 10 minutes, I restarted via --force push

@nicolas-grekas
Copy link
Member

can you still fix fabbot failure please?

@OskarStark
Copy link
Contributor Author

can you still fix fabbot failure please?

done @nicolas-grekas 👍

@OskarStark
Copy link
Contributor Author

Hmm, I don't know why they are failing... 😢

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.

travis error is a false positive

@OskarStark
Copy link
Contributor Author

travis error is a false positive

good to know, thank you @nicolas-grekas

fabpot added a commit that referenced this pull request Sep 30, 2017
This PR was squashed before being merged into the 2.7 branch (closes #24342).

Discussion
----------

removed useless PHPDoc

| Q             | A
| ------------- | ---
| Branch?       | 2.7
| 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 |
| License       | MIT
| Doc PR        | n/a

Commits
-------

5ee9043 removed useless PHPDoc
@fabpot fabpot closed this Sep 30, 2017
@OskarStark OskarStark deleted the patch-2 branch October 2, 2017 04:59
fabpot added a commit that referenced this pull request Oct 4, 2017
This PR was merged into the 3.4 branch.

Discussion
----------

[DI] remove inheritdoc from dumped container

| Q             | A
| ------------- | ---
| Branch?       | 3.4
| Bug fix?      | no
| New feature?  | no <!-- don't forget to update src/**/CHANGELOG.md files -->
| BC breaks?    | no
| Deprecations? | no <!-- don't forget to update UPGRADE-*.md files -->
| Tests pass?   | yes
| Fixed tickets |
| License       | MIT
| Doc PR        |

The inheritdoc without adding text is pointless on it's own as any decent IDE will show that those methods are overwriting the base method. This removes it from the dumped container to remove memory and generation time. This goes in the same direction as #23673, #18048 and #24342
It does not affect phpdocs of the compiled container that might actually be useful (factory methods of services explaining if the service is private, shared etc.).

Commits
-------

d779845 [DI] remove inheritdoc from dumped container
ostrolucky pushed a commit to ostrolucky/symfony that referenced this pull request Mar 25, 2018
This PR was squashed before being merged into the 2.7 branch (closes symfony#24342).

Discussion
----------

removed useless PHPDoc

| Q             | A
| ------------- | ---
| Branch?       | 2.7
| 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 |
| License       | MIT
| Doc PR        | n/a

Commits
-------

5ee9043 removed useless PHPDoc
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