Document the weak_vendors value #7453

greg0ire · 2017-02-07T22:24:35Z

greg0ire · 2017-02-07T22:25:37Z

nicolas-grekas

Should be more descriptive imho

nicolas-grekas · 2017-02-07T22:30:21Z

components/phpunit_bridge.rst

@@ -119,7 +119,25 @@ an arbitrary value (ex: ``320``) will make the tests fails only if a higher
 number of deprecation notices is reached (``0`` is the default value). You can
 also set the value ``"weak"`` which will make the bridge ignore any deprecation
 notices. This is useful to projects that must use deprecated interfaces for
-backward compatibility reasons.
+backward compatibility reasons, or for libraries with lots of vendors that


Lots of is undefined to the reader, it doesn't help one understand if that applies to its use cases or not

I guess one dependency with many deprecations is enough indeed.

nicolas-grekas · 2017-02-07T22:30:52Z

components/phpunit_bridge.rst

@@ -119,7 +119,25 @@ an arbitrary value (ex: ``320``) will make the tests fails only if a higher
 number of deprecation notices is reached (``0`` is the default value). You can
 also set the value ``"weak"`` which will make the bridge ignore any deprecation
 notices. This is useful to projects that must use deprecated interfaces for
-backward compatibility reasons.
+backward compatibility reasons, or for libraries with lots of vendors that
+can create a sudden backlog of deprecations to fix that would otherwise prevent


Same for sudden imho

I want to convey the fact that this can happen without any code change in the lib itself, because dependencies are not locked.

nicolas-grekas · 2017-02-07T22:31:34Z

components/phpunit_bridge.rst

-backward compatibility reasons.
+backward compatibility reasons, or for libraries with lots of vendors that
+can create a sudden backlog of deprecations to fix that would otherwise prevent
+unrelated pull requests from being merged… which leads us to ``"weak_vendors"``.


Ellipses suggest the reader understood, which might be not true yet

Replaced the ellipsis with described below so that the reader doesn't feel like they missed something.

nicolas-grekas · 2017-02-07T22:32:35Z

(but not longer ;) )

greg0ire · 2017-02-07T22:42:28Z

I reworked it a bit. Reviewers, feel free to point me to parts that you feel are too verbose / superfluous.

julienfalque · 2017-02-08T08:19:52Z

components/phpunit_bridge.rst

+the Composer lock file, which would create another class of issues. Libraries
+will often use ``SYMFONY_DEPRECATIONS_HELPER=weak`` because of this. This has
+the drawback of allowing contributions that introduce deprecations but:
+* forget to fix the deprecated calls if there are any;


Does not render as a bullet list because of missing new line.

Good catch! Fixed!

The list still doesn't render properly, see https://github.com/greg0ire/symfony-docs/blob/weak_vendors/components/phpunit_bridge.rst#making-tests-fail.

Woops, I think I amended a very old commit and force pushed it.

Fixed again

This PR was merged into the 3.3-dev branch. Discussion ---------- Introduce weak vendors mode Deprecations coming from the vendors are segregated from other deprecations. A new mode is introduced, in which deprecations coming from the vendors are not taken into account when deciding to exit with an error code. | Q | A | ------------- | --- | Branch? | master | Bug fix? | no | New feature? | no | BC breaks? | no | Deprecations? | no | Tests pass? | yes | License | MIT | Doc PR | symfony/symfony-docs#7453  Sample output : # Weak vendor mode ## With both vendors and non-vendor errors ``` WARNINGS! Tests: 1068, Assertions: 2714, Warnings: 6, Skipped: 15. Remaining deprecation notices (1) some error: 1x 1x in SonataAdminBundleTest::testBuild from Sonata\AdminBundle\Tests Remaining vendor deprecation notices (4) Legacy deprecation notices (48) ``` Exit code is 1 ## After fixing non-vendor errors ``` WARNINGS! Tests: 1068, Assertions: 2714, Warnings: 6, Skipped: 15. Remaining vendor deprecation notices (4) Legacy deprecation notices (48) ``` Exit code is 0 # TODO - [x] fix colorization issues (vendor deprecation notices are always in red) - [x] make the vendor detection more robust - [x] make the vendor dir configurable - [x] ~figure out how to run tests and~ add more of them - [x] test on non-vendor file - [x] test on vendor file - [x] do not change the output of other modes Commits ------- 61fd043 Introduce weak vendors mode

greg0ire · 2017-02-28T07:09:13Z

Please remove the [WCM] tag and review this

javiereguiluz

👍

Really well explained! Thanks @greg0ire

greg0ire · 2017-02-28T09:00:20Z

Glad you like it @javiereguiluz !

xabbuh · 2017-03-07T21:31:12Z

components/phpunit_bridge.rst

-backward compatibility reasons.
+backward compatibility reasons, or for libraries that do not want deprecations
+triggered by their dependencies to prevent unrelated pull requests from being
+merged, which leads us to the ``"weak_vendors"`` value, described below.


I would revert the changes to this paragraph.

Why? What's wrong with trying to make a smooth transition like that?

After more thinking, maybe you're right, I'm kind of explaining the same thing twice here.

xabbuh · 2017-03-13T08:51:51Z

Thank you @greg0ire.

This PR was merged into the master branch. Discussion ---------- Document the weak_vendors value See symfony/symfony#21539 Commits ------- abe88c6 Document the weak_vendors value

carsonbot added the Status: Needs Review label Feb 7, 2017

greg0ire mentioned this pull request Feb 7, 2017

Introduce weak vendors mode symfony/symfony#21539

Merged

7 tasks

nicolas-grekas reviewed Feb 7, 2017

View reviewed changes

greg0ire force-pushed the weak_vendors branch from 9b2e565 to b3b458b Compare February 7, 2017 22:40

julienfalque reviewed Feb 8, 2017

View reviewed changes

greg0ire force-pushed the weak_vendors branch from b3b458b to 629498f Compare February 8, 2017 08:58

javiereguiluz changed the title ~~Document the weak_vendors value~~ [WCM] Document the weak_vendors value Feb 12, 2017

javiereguiluz changed the title ~~[WCM] Document the weak_vendors value~~ Document the weak_vendors value Feb 28, 2017

javiereguiluz approved these changes Feb 28, 2017

View reviewed changes

xabbuh added Status: Reviewed and removed Status: Needs Review labels Mar 6, 2017

xabbuh added this to the 3.3 milestone Mar 6, 2017

xabbuh reviewed Mar 7, 2017

View reviewed changes

greg0ire force-pushed the weak_vendors branch from 629498f to bd84789 Compare March 7, 2017 22:00

Document the weak_vendors value

abe88c6

greg0ire force-pushed the weak_vendors branch from bd84789 to abe88c6 Compare March 7, 2017 22:14

xabbuh merged commit abe88c6 into symfony:master Mar 13, 2017

greg0ire deleted the weak_vendors branch March 13, 2017 09:31

greg0ire mentioned this pull request Mar 14, 2017

[PhpUnitBridge] weak vendors mode silence legitimate deprecations symfony/symfony#21991

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Document the weak_vendors value #7453

Document the weak_vendors value #7453

greg0ire commented Feb 7, 2017 •

edited by javiereguiluz

Loading

greg0ire commented Feb 7, 2017

nicolas-grekas left a comment

nicolas-grekas Feb 7, 2017

greg0ire Feb 7, 2017 •

edited

Loading

nicolas-grekas Feb 7, 2017

greg0ire Feb 7, 2017

nicolas-grekas Feb 7, 2017

greg0ire Feb 7, 2017

nicolas-grekas commented Feb 7, 2017

greg0ire commented Feb 7, 2017

julienfalque Feb 8, 2017

greg0ire Feb 8, 2017

julienfalque Mar 7, 2017

greg0ire Mar 7, 2017

greg0ire Mar 7, 2017

greg0ire commented Feb 28, 2017

javiereguiluz left a comment

greg0ire commented Feb 28, 2017

xabbuh Mar 7, 2017

greg0ire Mar 7, 2017

greg0ire Mar 7, 2017

xabbuh commented Mar 13, 2017

Document the weak_vendors value #7453

Document the weak_vendors value #7453

Conversation

greg0ire commented Feb 7, 2017 • edited by javiereguiluz Loading

greg0ire commented Feb 7, 2017

nicolas-grekas left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

greg0ire Feb 7, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

nicolas-grekas commented Feb 7, 2017

greg0ire commented Feb 7, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

greg0ire commented Feb 28, 2017

javiereguiluz left a comment

Choose a reason for hiding this comment

greg0ire commented Feb 28, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

xabbuh commented Mar 13, 2017

greg0ire commented Feb 7, 2017 •

edited by javiereguiluz

Loading

greg0ire Feb 7, 2017 •

edited

Loading