Movatterモバイル変換


[0]ホーム

URL:


Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

[PhpUnitBridge] More accurate grouping#31730

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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Merged
fabpot merged 1 commit intosymfony:4.3fromgreg0ire:more-accurate-grouping
Jun 26, 2019

Conversation

@greg0ire
Copy link
Contributor

QA
Branch?4.3
Bug fix?yes
New feature?no
BC breaks?no
Deprecations?no
Tests pass?yes
Fixed ticketsn/a
LicenseMIT
Doc PRn/a

Sometimes, you cannot easily know if code was written by a vendor or
directly in the application, for instance if the code comes from a file
in the cache. In that case, it is better not to classify the deprecation
as direct or indirect.

@jmsche please test this on your application when you can, I think you might be having that issue.

yceruto, jmsche, and jeroennoten reacted with thumbs up emoji
@jmsche
Copy link
Contributor

Hi there, it seems better to me :)

FYI using Symfony 4.3.0-beta1 (without the Twig fixes inform_div_layout.html.twig) here are the results:

Pre-patch:

Remaining direct deprecation notices (1)  1x: Using an "if" condition on "for" tag is deprecated since Twig 2.10.0, use a "filter" filter or an "if" condition inside the "for" body instead (if your condition depends on a variable updated inside the loop).    1x in EventControllerTest::testIndex from App\Tests\Controller\Admin

Post-patch:

Other deprecation notices (1)  1x: Using an "if" condition on "for" tag is deprecated since Twig 2.10.0, use a "filter" filter or an "if" condition inside the "for" body instead (if your condition depends on a variable updated inside the loop).    1x in EventControllerTest::testIndex from App\Tests\Controller\Admin
greg0ire reacted with hooray emoji

@greg0iregreg0ireforce-pushed themore-accurate-grouping branch 2 times, most recently froma2a7612 to0febb03CompareMay 30, 2019 13:31
@greg0iregreg0ire marked this pull request as ready for reviewMay 30, 2019 14:27
@greg0iregreg0ireforce-pushed themore-accurate-grouping branch 2 times, most recently from116907f tod67f50cCompareMay 30, 2019 14:31
{
privateconstPATH_TYPE_VENDOR ='path_type_vendor';
privateconstPATH_TYPE_SELF ='path_type_internal';
privateconstPATH_TYPE_UNDETERMINED ='path_type_undetermined';
Copy link
ContributorAuthor

Choose a reason for hiding this comment

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

I could have used an exception instead of 3-value return, but it did not feel "exceptional" enough. This is going to happen whenever a deprecation comes from the cache.

Choose a reason for hiding this comment

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

hello PHP 5.5, we cannot use these consts

Choose a reason for hiding this comment

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

fixed in0c9b3c0

@chalasrchalasr added this to the4.3 milestoneMay 30, 2019
@nicolas-grekasnicolas-grekas changed the titleMore accurate grouping[PhpUnitBridge] More accurate groupingMay 31, 2019
@fabpotfabpotforce-pushed themore-accurate-grouping branch fromd67f50c tod9f0ba3CompareJune 26, 2019 08:11
@fabpot
Copy link
Member

Thank you@greg0ire.

@fabpotfabpot merged commitd9f0ba3 intosymfony:4.3Jun 26, 2019
fabpot added a commit that referenced this pull requestJun 26, 2019
This PR was squashed before being merged into the 4.3 branch (closes#31730).Discussion----------[PhpUnitBridge] More accurate grouping| Q             | A| ------------- | ---| Branch?       | 4.3| Bug fix?      | yes| New feature?  | no| BC breaks?    | no| Deprecations? | no| Tests pass?   | yes| Fixed tickets | n/a| License       | MIT| Doc PR        | n/aSometimes, you cannot easily know if code was written by a vendor ordirectly in the application, for instance if the code comes from a filein the cache. In that case, it is better not to classify the deprecationas direct or indirect.@jmsche please test this on your application when you can, I think you might be having that issue.Commits-------d9f0ba3 [PhpUnitBridge] More accurate grouping
@greg0iregreg0ire deleted the more-accurate-grouping branchJune 26, 2019 09:43
@nicolas-grekas
Copy link
Member

This breaks the CI and I cannot reproduce locally (yet), for deps=low/high jobs:
https://travis-ci.org/symfony/symfony/jobs/550756898

Not a bug, just a side-effect of the CI.
I'm going to revert for now, please submit again.

greg0ire reacted with thumbs up emoji

@nicolas-grekas
Copy link
Member

Reverted in4814fd3

nicolas-grekas added a commit that referenced this pull requestJun 26, 2019
nicolas-grekas added a commit that referenced this pull requestJun 26, 2019
* 4.3:  Revert "bug#31730 [PhpUnitBridge] More accurate grouping (greg0ire)"
nicolas-grekas added a commit that referenced this pull requestJun 26, 2019
* 4.4:  Revert "bug#31730 [PhpUnitBridge] More accurate grouping (greg0ire)"
nicolas-grekas added a commit that referenced this pull requestJun 26, 2019
nicolas-grekas added a commit that referenced this pull requestJun 26, 2019
* 4.3:  Reject phpunit-bridge v5 for now  Revert "Revert "bug#31730 [PhpUnitBridge] More accurate grouping (greg0ire)""
nicolas-grekas added a commit that referenced this pull requestJun 26, 2019
* 4.4:  Reject phpunit-bridge v5 for now  Revert "Revert "bug#31730 [PhpUnitBridge] More accurate grouping (greg0ire)""
@nicolas-grekas
Copy link
Member

Revert reverted because actually it's not the issue, I'm loosing my mind on because I cannot reproduce locally :)

greg0ire reacted with laugh emoji

@fabpotfabpot mentioned this pull requestJun 26, 2019
Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment

Reviewers

@nicolas-grekasnicolas-grekasnicolas-grekas left review comments

@fabpotfabpotfabpot approved these changes

Assignees

No one assigned

Projects

None yet

Milestone

4.3

Development

Successfully merging this pull request may close these issues.

6 participants

@greg0ire@jmsche@fabpot@nicolas-grekas@chalasr@carsonbot

[8]ページ先頭

©2009-2025 Movatter.jp