Movatterモバイル変換

carsonbot added this to the5.2 milestone

Copy link

carsonbot commentedDec 14, 2020

Hey!

I see that this is your first PR. That is great! Welcome!

Symfony has acontribution guide which I suggest you to read.

In short:

Always add tests
Keep backward compatibility (seehttps://symfony.com/bc).
Bug fixes must be submitted against the lowest maintained branch where they apply (seehttps://symfony.com/releases)
Features and deprecations must be submitted against the 5.x branch.

Review the GitHub status checks of your pull request and try to solve the reported issues. If some tests are failing, try to see if they are failing because of this change.

When two Symfony core team members approve this change, it will be merged and you will become an official Symfony contributor!
If this PR is merged in a lower version branch, it will be merged up to all maintained branches within a few days.

I am going to sit back now and wait for the reviews.

Cheers!

Carsonbot

guillaume-sainthillier changed the title~~Fix #39468 issue with date immutable~~[DoctrineBridge] Fix bug with date immutable and datetime immutable

nicolas-grekas reviewed

src/Symfony/Bridge/Doctrine/Tests/Form/DoctrineOrmTypeGuesserTest.php

Copy link

Member

nicolas-grekas left a comment

Choose a reason for hiding this comment

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

Thanks. I suppose#39469 should be closed?

		class DoctrineOrmTypeGuesserTestextends TestCase
		{
		/**
		* @dataProvider requiredType

Copy link

Member

nicolas-grekasDec 14, 2020

Choose a reason for hiding this comment

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

no need for a provider here since there is only one item in the provided list

Copy link

ContributorAuthor

guillaume-sainthillierDec 14, 2020

Choose a reason for hiding this comment

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

Do you want me to add more items to this list?

Copy link

Member

nicolas-grekasDec 14, 2020

Choose a reason for hiding this comment

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

only if it makes sense to improve the code coverage
otherwise, no need for the provider

Copy link

ContributorAuthor

guillaume-sainthillierDec 14, 2020

Choose a reason for hiding this comment

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

I've added one more test to also cover DateTimeImmutable :-)

Copy link

ContributorAuthor

guillaume-sainthillier commentedDec 14, 2020

Yes I will close other PR then

Added one more test

2b8b5a4

jderusse reviewed

src/Symfony/Bridge/Doctrine/Tests/Form/DoctrineOrmTypeGuesserTest.php OutdatedShow resolvedHide resolved

Update src/Symfony/Bridge/Doctrine/Tests/Form/DoctrineOrmTypeGuesserT…

b612f5e

…est.phpCo-authored-by: Jérémy Derussé <jeremy@derusse.com>

jderusse approved these changes

Dec 15, 2020

carsonbot added Status: Reviewed and removed Status: Needs Review labels

Dec 15, 2020

derrabus approved these changes

derrabus changed the title~~[DoctrineBridge] Fix bug with date immutable and datetime immutable~~[DoctrineBridge] Guess correct form types for DATE_IMMUTABLE and DATETIME_IMMUTABLE

Copy link

Member

derrabus commentedDec 16, 2020

Thank you@guillaume-sainthillier.

derrabus added a commit that referenced this pull request

bug#39498[DoctrineBridge] Guess correct form types for DATE_IMMUTAB…

6112be3

…LE and DATETIME_IMMUTABLE (guillaume-sainthillier)This PR was squashed before being merged into the 5.2 branch.Discussion----------[DoctrineBridge] Guess correct form types for DATE_IMMUTABLE and DATETIME_IMMUTABLE| Q             | A| ------------- | ---| Branch?       | 5.2| Bug fix?      | yes| New feature?  | no| Deprecations? | no| Tickets       |Fix#39468| License       | MIT| Doc PR        | -Rebased PR#39469Fixes#39468 related to Doctrine Form Type Guesser with DateImmutable typeCommits-------238d318 [DoctrineBridge] Guess correct form types for DATE_IMMUTABLE and DATETIME_IMMUTABLE

derrabus closed this