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

[Mime] Removes deprecated tests#41404

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

Conversation

@0x346e3730
Copy link
Contributor

@0x346e37300x346e3730 commentedMay 24, 2021•
edited
Loading

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

Follow-up from#41398 to fix CI for 6.0.

@nicolas-grekas
Copy link
Member

Sorry I wasn't clear enough. On legacy tests, calling deprecated code is expected! This PR is for 6.0:)

@0x346e37300x346e3730force-pushed themime-remove-deprecated-tests branch from679de60 tocc358f4CompareMay 24, 2021 21:41
@0x346e37300x346e3730 changed the base branch from5.2 to6.0May 24, 2021 21:41
@0x346e3730
Copy link
ContributorAuthor

My bad, I hesitated and went for it as it's late, should be good now :) Not sure why it auto asked for review tho, sorry for the unnecessary pings :(

Copy link
Member

@NyholmNyholm left a comment

Choose a reason for hiding this comment

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

I think this looks good.

Address::fromString() is removed in favour ofAddress::create(). These tests are no longer needed.

@0x346e3730
Copy link
ContributorAuthor

I just reviewed as I've sent the PR late yesterday, shouldn't I also delete fromStringProvider ?

@0x346e3730
Copy link
ContributorAuthor

I just reviewed as I've sent the PR late yesterday, shouldn't I also delete fromStringProvider ?

It's still used in testCreateWithString, I don't know if renaming the provider is a good idea or not.

@Nyholm
Copy link
Member

Good observation.

Let's rename it. There is no backwards compatibility issue with files in/Tests/

@nicolas-grekas
Copy link
Member

The rename should happen on 5.3 then!

@nicolas-grekas
Copy link
Member

Or better: don't rename

@Nyholm
Copy link
Member

Oh... let's not rename it then =)

Im happy with this PR in its current state.

@0x346e3730
Copy link
ContributorAuthor

I'm cool with not renaming it too.

@Nyholm
Copy link
Member

Awesome. Congratulations to your first contribution!

0x346e3730 reacted with heart emoji

@NyholmNyholm merged commit8885c2d intosymfony:6.0May 25, 2021
Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment

Reviewers

@nicolas-grekasnicolas-grekasnicolas-grekas approved these changes

@NyholmNyholmNyholm approved these changes

Assignees

No one assigned

Projects

None yet

Milestone

6.0

Development

Successfully merging this pull request may close these issues.

4 participants

@0x346e3730@nicolas-grekas@Nyholm@carsonbot
[8]ページ先頭
©2009-2025 Movatter.jp