Skip to content

Remove obsolete versionadded in 3.4 #11080

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

OskarStark
Copy link
Contributor

No description provided.

OskarStark and others added 6 commits March 1, 2019 14:33
This PR was submitted for the master branch but it was merged into the 3.4 branch instead (closes symfony#11073).

Discussion
----------

use correct code-block

<!--

If your pull request fixes a BUG, use the oldest maintained branch that contains
the bug (see https://symfony.com/roadmap for the list of maintained branches).

If your pull request documents a NEW FEATURE, use the same Symfony branch where
the feature was introduced (and `master` for features of unreleased versions).

-->

Commits
-------

7ff8c9b use correct code-block
This PR was merged into the 3.4 branch.

Discussion
----------

Minor "Review comments" tweaks

Just some minor consistency tweaks & typo fixes :)

Commits
-------

a1bd4ee Minor "Review comments" tweaks
…behaves like version… (OskarStark)

This PR was merged into the 3.4 branch.

Discussion
----------

removed versionadded, fixed count and note which behaves like version…

…added

<!--

If your pull request fixes a BUG, use the oldest maintained branch that contains
the bug (see https://symfony.com/roadmap for the list of maintained branches).

If your pull request documents a NEW FEATURE, use the same Symfony branch where
the feature was introduced (and `master` for features of unreleased versions).

-->

Commits
-------

04ac2c7 removed versionadded, fixed count and note which behaves like versionadded
@OskarStark OskarStark force-pushed the remove-obsolete-versionadded-in-3.4 branch from c1bfae8 to 8978214 Compare March 4, 2019 12:19
@OskarStark OskarStark force-pushed the remove-obsolete-versionadded-in-3.4 branch from 8978214 to 914500e Compare March 4, 2019 12:21
@javiereguiluz
Copy link
Member

Is it possible that this PR has some wrong commits? It looks a bit strange now. Thanks.

@OskarStark
Copy link
Contributor Author

Will check later 👍🏻

Copy link
Contributor

@HeahDude HeahDude left a comment

Choose a reason for hiding this comment

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

Apart minor comments, big 👍 from me, thanks!

@@ -91,34 +91,34 @@ Don't use hyperbole ("always", "never", "endlessly", "nothing", "worst", "horrib
**Don't:** *"I don't like how you wrote this code"* - there is no clear explanation why you
don't like how it's written.

**Better:** *"I find it hard to read this code as there many nested if statements, can you make it more
readable? By encapsulating some of it's details or maybe adding some comments to explain the overall logic."* -
**Better:** *"I find it hard to read this code as there is many nested if statements, can you make it more
Copy link
Contributor

Choose a reason for hiding this comment

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

there are

**Better:** *"I find it hard to read this code as there many nested if statements, can you make it more
readable? By encapsulating some of it's details or maybe adding some comments to explain the overall logic."* -
**Better:** *"I find it hard to read this code as there is many nested if statements, can you make it more
readable? By encapsulating some of its details or maybe adding some comments to explain the overall logic."* -
Copy link
Contributor

Choose a reason for hiding this comment

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

the?


* ``Symfony 3 still uses PHP 5 and doesn't allow the usage scalar type-hints.``.
* "Symfony 3 still uses PHP 5 and doesn't allow the usage scalar type-hints."
Copy link
Contributor

Choose a reason for hiding this comment

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

the usage of


* Code doesn't match Symfony's CS rules (e.g. use ``[]`` instead of ``array()``)
* "Code doesn't match Symfony's CS rules (e.g. use ``[]`` instead of ``array()``)"
Copy link
Contributor

Choose a reason for hiding this comment

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

Can be removed now I guess

@HeahDude
Copy link
Contributor

HeahDude commented Mar 5, 2019

Also we should target 3.4 here.

@HeahDude HeahDude added this to the 3.4 milestone Mar 5, 2019
@HeahDude
Copy link
Contributor

HeahDude commented Mar 5, 2019

Then remove useless notices in another PR targetting 4.2.

@OskarStark OskarStark mentioned this pull request Mar 6, 2019
@OskarStark
Copy link
Contributor Author

Is it possible that this PR has some wrong commits? It looks a bit strange now. Thanks.

Hmm you are right, lets close it for now, I will check that

@OskarStark OskarStark closed this Mar 6, 2019
@OskarStark OskarStark deleted the remove-obsolete-versionadded-in-3.4 branch March 6, 2019 11:14
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.

6 participants