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

Remove wrong @group legacy annotations#34433

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
nicolas-grekas merged 1 commit intosymfony:4.4fromnicolas-grekas:wrong-legacy
Nov 20, 2019

Conversation

@nicolas-grekas
Copy link
Member

QA
Branch?4.4
Bug fix?no
New feature?no
Deprecations?no
Tickets-
LicenseMIT
Doc PR-

These annotations are still found on branch 5.0.
Does this mean they are wrong? Why don't they make 5.0 fail if not?

@nicolas-grekas
Copy link
MemberAuthor

1x: Using names for buttons that do not start with a lowercase letter, a digit, or an underscore is deprecated since Symfony 4.3 and will throw an exception in 5.0 ("Button" given).

but it doesn't throw on 5.0

  1. Symfony\Component\Routing\Tests\Loader\ObjectLoaderTest::testExceptionOnNoObjectReturned
    Failed asserting that exception of type "TypeError" matches expected exception "LogicException". Message was: "Return value of Symfony\Component\Routing\Tests\Loader\TestObjectLoader::getObject() must be an object, string returned" at ObjectLoaderTest.php:111

that's the only other failure triggered by this change.

This means we should have a look twice at these two tests on master, and we should remove all the other annotations since they're not covering any deprecations.

Anyone?

];
}

/**
Copy link
Contributor

Choose a reason for hiding this comment

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

This one should be removed here and on master 👍

Copy link
Member

@lyrixxlyrixx left a comment

Choose a reason for hiding this comment

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

LGTM

@fancyweb
Copy link
Contributor

@nicolas-grekas Looks like we will have to applyhttps://github.com/symfony/symfony/pull/34434/files#diff-40e4c90663826378950ff96dce5b445eR55 on 4.4 as well or skip the test.

@nicolas-grekas
Copy link
MemberAuthor

Looks like we will have to applyhttps://github.com/symfony/symfony/pull/34434/files#diff-40e4c90663826378950ff96dce5b445eR55 on 4.4 as well or skip the test.

applied

}

/**
* @group legacy
Copy link
Member

Choose a reason for hiding this comment

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

Removing this was missed in#33507 where the deprecation of theintercept_redirects option was reverted.

}

/**
* @group legacy
Copy link
Member

Choose a reason for hiding this comment

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

Removing this was missed in#33507 where the deprecation of theintercept_redirects option was reverted.

nicolas-grekas added a commit that referenced this pull requestNov 20, 2019
This PR was merged into the 4.4 branch.Discussion----------Remove wrong@group legacy annotations| Q             | A| ------------- | ---| Branch?       | 4.4| Bug fix?      | no| New feature?  | no| Deprecations? | no| Tickets       | -| License       | MIT| Doc PR        | -These annotations are still found on branch 5.0.Does this mean they are wrong? Why don't they make 5.0 fail if not?Commits-------8d84ac3 Remove wrong@group legacy annotations
@nicolas-grekasnicolas-grekas merged commit8d84ac3 intosymfony:4.4Nov 20, 2019
@nicolas-grekasnicolas-grekas deleted the wrong-legacy branchNovember 20, 2019 13:27
Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment

Reviewers

@stofstofstof left review comments

@lyrixxlyrixxlyrixx approved these changes

@xabbuhxabbuhxabbuh approved these changes

+1 more reviewer

@fancywebfancywebfancyweb left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Projects

None yet

Milestone

4.4

Development

Successfully merging this pull request may close these issues.

6 participants

@nicolas-grekas@fancyweb@lyrixx@stof@xabbuh@carsonbot

[8]ページ先頭

©2009-2025 Movatter.jp