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

[DoctrineBridge] Fix bug with date immutable and datetime immutable#39469

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

Closed
FlorianM11 wants to merge107 commits intosymfony:5.2fromsilarhi:5.x

Conversation

@FlorianM11
Copy link

@FlorianM11FlorianM11 commentedDec 11, 2020
edited by nicolas-grekas
Loading

QA
Branch?5.2
Bug fix?yes
New feature?no
Deprecations?no
TicketsFix#39468
LicenseMIT
Doc PR-

nicolas-grekasand others added30 commitsNovember 4, 2020 16:35
…l-intl-icu (nicolas-grekas)This PR was merged into the 5.x branch.Discussion----------[Intl] deprecate polyfills in favor of symfony/polyfill-intl-icu| Q             | A| ------------- | ---| Branch?       | 5.x| Bug fix?      | no| New feature?  | no| Deprecations? | yes| Tickets       |Fix#37758| License       | MIT| Doc PR        | -Followssymfony/polyfill#310/cc@stofCommits-------6ad0169 [Intl] deprecate polyfills in favor of symfony/polyfill-intl-icu
* 5.2:  Bump branch-version  Bump Symfony version to 5.2.0  Update VERSION for 5.2.0-RC1  Update CHANGELOG for 5.2.0-RC1
…Nyholm)This PR was squashed before being merged into the 5.3-dev branch.Discussion----------[Messenger][SQS] Make sure one can enable debug logs| Q             | A| ------------- | ---| Branch?       | 5.x| Bug fix?      | no| New feature?  | yes| Deprecations? | no| Tickets       || License       | MIT| Doc PR        | NeededIf you add `&debug=true` on your DSN, then we will use `LoggerInterface::debug()` to print HTTP requests and responses.This has a negative impact on performance, but it will be helpful when debugging.Commits-------66edc59 [Messenger][SQS] Make sure one can enable debug logs
…it client (alexander-schranz)This PR was squashed before being merged into the 5.3-dev branch.Discussion----------[BrowserKit] Add jsonRequest function to the browser-kit client| Q             | A| ------------- | ---| Branch?       | 5.x| Bug fix?      | no| New feature?  | yes| Deprecations? | no| Tickets       | Fix -| License       | MIT| Doc PR        | symfony/symfony-docs#...If you use the FOSRestBundle for your Api's you have maybe many tests using just:```php$client->request('POST', '/api/contacts', ['param' => 1]);```To test your JSON api as in the real browser request FOSRestBundle converts the json body into the `$request->request` object. I think something similar is done by ApiPlatform. If you have tests like above they will now fail as the integer is converted to string see also#38591.This PR add a new `jsonRequest` which will look like the following and will so fix the above problem:```php$client->jsonRequest('POST', '/api/contacts', ['param' => 1]);```Commits-------c2fa2cb [BrowserKit] Add jsonRequest function to the browser-kit client
* 5.2:  [Filesystem] fix cleaning up tmp files when dumpFile() fails  [MimeType] Add missing alias for @mime_type
* 5.2:  Add tests on CacheDataCollector  [ProxyManagerBridge] fix tests  [ProxyManagerBridge] relax fixture in tests  Fix circular detection with multiple paths
* 5.2:  prevent hash collisions caused by reused object hashes  autoconfigure behavior describing tags on decorators  [Validator][RecursiveContextualValidator] Prevent validated hash collisions
…sport injectable (jacekwilczynski)This PR was merged into the 5.3-dev branch.Discussion----------[Messenger] Make all the dependencies of AmazonSqsTransport injectable| Q             | A| ------------- | ---| Branch?       | 5.x for features| Bug fix?      | no| New feature?  | yes - updated changelog| Deprecations? | no| Tickets       |Fix#38640| License       | MIT| Doc PR        | symfony/symfony-docs#... <!-- required for new features -->This is a pure refactoring PR that enables more flexibility with service injection without actually changing any behaviour or breaking backwards compatibility. It satisfies only 1 of 2 acceptance criteria of#38640 but since they're independent, I'm not marking the PR as WIP.## Receiver & sender injection into AmazonSqsTransportIt is now possible to inject your own receiver and sender into `Symfony\Component\Messenger\Bridge\AmazonSqs\Transport\AmazonSqsTransport`.### Recommended way - AmazonSqsTransport::createFor clean dependency injection, I recommed using the `create` static method, which obliges you to pass all dependencies:```php$transport = AmazonSqsTransport::create($connection, $receiver, $sender);```For example, this code from `Symfony\Component\Messenger\Bridge\AmazonSqs\Transport\AmazonSqsTransportFactory`:```phpreturn new AmazonSqsTransport(Connection::fromDsn($dsn, $options), $serializer);```could be replaced with this:```php$connection = Connection::fromDsn($dsn, $options);return AmazonSqsTransport::create(    $connection,    new AmazonSqsReceiver($connection, $serializer),    new AmazonSqsSender($connection, $serializer));```I didn't replace that code in the factory because I didn't find it essential but I will certainly do it in my custom factory in my project, passing my own receiver implementation.### Using the main constructorYou can still use the main constructor but it's most suited for backwards compatibility, i.e. when you don't want to inject a receiver or a sender. With the full list of arguments it gets a bit messy due to their optionality.#### Minimal call```phpnew AmazonSqsTransport($connection);```As before this PR, a receiver and a sender will be created using the default serializer, i.e. `Symfony\Component\Messenger\Transport\Serialization\PhpSerializer`.#### With a custom serializer```phpnew AmazonSqsTransport($connection, $serializer);```As before this PR, a receiver and a sender will be created using the passed serializer.#### With a custom receiver and sender```phpnew AmazonSqsTransport($connection, null, $receiver, $sender);```The injected services will be used. The second parameter (serializer) is unnecessary because it was only ever used while creating a receiver and a sender inside the transport. Because of this, I recommend using the new static `create` method.Commits-------281af26 [Messenger] Make all the dependencies of AmazonSqsTransport injectable
* 5.2:  remove unreachable code  [Browserkit] Add changelog entry for request parameters string cast  Update ExceptionEvent.php  fix firebase transport factory DI tag type  [Validator] Resolve IsinValidator's dependency on the validator.  [HttpFoundation] Fix for virtualhosts based on URL path
* 5.2:  [Ldap] Fix undefined variable $con.  Use GithubAction to run ldap tests  Adds LDAP Adapter test in integration group  Fix critical extension when reseting paged control  Reinitialize globBrace after unserialization
* 5.2:  do not depend on the actual time to fix a transient test  Run Redis Sentinel tests in GithubAction  Minor : Removed typo (extra "the" term)  Check if method inheritEnvironmentVariables exists  [PhpUnitBridge] Fix test fixture file name
…for errors (ogizanagi)This PR was squashed before being merged into the 5.3-dev branch.Discussion----------[Console][Yaml] Linter: add Github annotations format for errors| Q             | A| ------------- | ---| Branch?       | 5.x <!-- see below -->| Bug fix?      | no| New feature?  | yes <!-- please update src/**/CHANGELOG.md files -->| Deprecations? | no <!-- please update UPGRADE-*.md and src/**/CHANGELOG.md files -->| Tickets       | N/A <!-- prefix each issue number with "Fix #", no need to create an issue if none exist, explain below instead -->| License       | MIT| Doc PR        | TODOGithub actions [can write errors and warning](https://docs.github.com/en/free-pro-team@latest/actions/reference/workflow-commands-for-github-actions#setting-an-error-message) directly in their output, which result into annotations into the Github checks. It can even provide a filename, line & col number, allowing to display the annnotations inside the PR diff directly, at the right place.More advanced usage of annotations can be made using the [API](https://docs.github.com/en/free-pro-team@latest/rest/reference/checks#list-check-run-annotations), but regarding the linters provided in Symfony components, it seems the shortcut using output is a great way to enhance the integration with Github Actions.This PR starts by proposing these changes in the yaml linter:- add the `github` format, which is the same as the `txt` one, except for errors and warning, for which we'll adapt the output to the Github annotations format.- remove the `txt` format as default, and autodetect if the script is running in a Github action context, then use `github` format. If it's not, we fallback to `txt` as before.Once we agree on the details, we could perform the same for other linters (xliff, twig, ...)Here is a PR using it:ogizanagi/symfony-lint-gha-demo#2and some screenshots:| PR checks run | PR checks annotations | PR diff || -- | -- | -- || ![Capture d’écran 2020-11-04 à 09 37 07](https://user-images.githubusercontent.com/2211145/98089377-ed416600-1e82-11eb-8b10-40602b45efb1.png) | ![Capture d’écran 2020-11-04 à 09 37 28](https://user-images.githubusercontent.com/2211145/98089379-edd9fc80-1e82-11eb-8302-4e104abaeb2c.png) | ![Capture d’écran 2020-11-04 à 09 38 28](https://user-images.githubusercontent.com/2211145/98089381-edd9fc80-1e82-11eb-982a-9e4413ec30ba.png) |~~(tests to add)~~---This was inspired by [PHPStan](https://github.com/phpstan/phpstan-src/blob/d77bd87da9f2fad0440fc1614158cdfc1b7cc88a/src/Command/ErrorFormatter/GithubErrorFormatter.php) which is already auto-adapting the output according to the CI, usinghttps://github.com/OndraM/ci-detectorCommits-------f0bbdc8 [Console][Yaml] Linter: add Github annotations format for errors
…age (tyx)This PR was merged into the 5.3-dev branch.Discussion----------[Messenger]  Allow InMemoryTransport to serialize message| Q             | A| ------------- | ---| Branch?       | 5.x| Bug fix?      | no| New feature?  | yes| Deprecations? | no| Tickets       |Fix#38893| License       | MIT| Doc PR        | wipThis introduce a query parameter in dsn to enable serialization as@Nyholm has suggested in#38893`in-memory://?serialize=true`Commits-------46a8007 [Messenger] Allow InMemoryTransport to serialize message
* 5.2:  Bump Symfony version to 5.2.0  Update VERSION for 5.2.0-RC2  Update CHANGELOG for 5.2.0-RC2  Display debug info  [HttpFoundation] Typo on deprecation package name  [DoctrineBridge] drop binary variants of UID types  [HttpClient] don't fallback to HTTP/1.1 when HTTP/2 streams break  Default to user provider, if available, in password upgrader  fix lexing nested sequences/mappings
* 5.2:  Fix test.  [PhpUnitBridge] Fix qualification of deprecations triggered by the debug class loader  Improve return phpdoc for Normalizer  Use a partial buffer in SymfonyStyle  Fix console closing tag  Fix typo in comment  [VarDumper] fix casting resources turned into objects on PHP 8  Removing AnonymousPassport
…heGarious)This PR was merged into the 5.3-dev branch.Discussion----------[Console] Added Invalid constant into Command Class| Q             | A| ------------- | ---| Branch?       | 5.x| Bug fix?      | no| New feature?  | no <!-- please update src/**/CHANGELOG.md files -->| Deprecations? | no <!-- please update UPGRADE-*.md and src/**/CHANGELOG.md files -->| License       | MIT<!--Replace this notice by a short README for your feature/bugfix. This will help peopleunderstand your PR and can be used as a start for the documentation.Additionally (seehttps://symfony.com/releases): - Always add tests and ensure they pass. - Never break backward compatibility (seehttps://symfony.com/bc). - Bug fixes must be submitted against the lowest maintained branch where they apply   (lowest branches are regularly merged to upper ones so they get the fixes too.) - Features and deprecations must be submitted against branch 5.x.-->Regarding into this [link](https://www.gnu.org/software/bash/manual/html_node/Exit-Status.html#:~:text=A%20non%2Dzero%20exit%20status,N%20as%20the%20exit%20status), we have 3 exits status standard :- 0 is for success- 1 is for error- 2 is for _indicate incorrect usage, generally invalid options or missing arguments_I think if we use a constant into Command class, we need to implement invalid exit status too.Commits-------449147b Added Invalid constant into Command Class
…(karlshea)This PR was merged into the 5.3-dev branch.Discussion----------[Ldap] Ldap Entry case-sensitive attribute key option| Q             | A| ------------- | ---| Branch?       | 5.x| Bug fix?      | no| New feature?  | yes| Deprecations? | no| Tickets       | N/A| License       | MIT| Doc PR        | N/ASee PR#36432Commits-------d3b9440 [Ldap] Ldap Entry case-sensitive attribute key option
… fabpot)This PR was merged into the 5.3-dev branch.Discussion----------[DomCrawler] Cache discovered namespaces| Q             | A| ------------- | ---| Branch?       | 5.x| Bug fix?      | no| New feature?  | yes| Deprecations? | no| Tickets       | Address#39067| License       | MITDiscovering namespaces is by far the most expensive task when filtering for nodes and the xpath contains prefixes.When `Crawler::filterRelativeXPath` is called multiple times with (identical) prefixes the slowdown is huge.This fix brings the runtime of the linked ticket down from 27 seconds to 9 seconds in my test. Compared to a pure PHP version which takes < 0.5 seconds the design of the crawler API is the limiting factor. There are still many repeated namespace queries caused by new Crawler instances. Ideas to solve this are discussed in the ticket.Commits-------a8e85ec Make some CS changes4c74dea Cache discovered namespaces in DomCrawler
ferrastasand others added13 commitsDecember 10, 2020 16:38
Both classes have an optional argument `$readTimeout` that can be set duringinitialization for `\RedisSentinel` and during `connect`/`pconnect`respectively.
… `\Redis` (ferrastas)This PR was merged into the 5.3-dev branch.Discussion----------[Cache] Make use of `read_timeout` in `\RedisSentinel` and `\Redis`| Q             | A| ------------- | ---| Branch?       | 5.x| Bug fix?      | yes| New feature?  | no| Deprecations? | no| Tickets       |Fix#39429| License       | MITThis is a bugfix for#39363 a feature introduced in 5.xAs described in issue#39429, `\RedisSentinel` accepts an optional read timeout value during construction.`read_timeout` is already part of the connection options, this PR just make use of it.Commits-------14e36a2 [Cache] Make use of `read_timeout` in `\RedisSentinel` and `\Redis`
…wing the exception (renan)This PR was merged into the 5.3-dev branch.Discussion----------[Cache] Bugfix provide the correct host and port when throwing the exception| Q             | A| ------------- | ---| Branch?       | 5.x| Bug fix?      | yes| New feature?  | no| Deprecations? | no| Tickets       | none| License       | MIT| Doc PR        | noneThe message before was including the result of the `getMasterAddrByName` call of which ended with:```Failed to retrieve master information from master name "mymaster" and address ":0".```Commits-------276bbb5 [Cache] Bugfix provide the correct host and port when throwing the exception
* 5.2:  Fix licence  Fix CS in link binary  [Cache] remove no-op  Fix CS in Changelogs  [Notifier][Sinch] Add tests  [Notifier] [Nexmo] Add tests  [Notifier][OvhCloud] Add tests  [Notifier] [Free Mobile] Rename method to match other bridges  [Cache] fix setting "read_timeout" when using Redis  Fix CS in changelogs  [Notifier] Streamline README files
…n handling of a message failed (loevgaard)This PR was squashed before being merged into the 5.3-dev branch.Discussion----------[Messenger] Added more descriptive exception message when handling of a message failed| Q             | A| ------------- | ---| Branch?       | 4.4| Bug fix?      | no (more of a patch thing)| New feature?  | no| Deprecations? | no| License       | MITI use Symfony Messenger extensively and I run into the `HandlerFailedException` from time to time. What bothers me is that the exception doesn't carry the name of the message that failed right there in the exception message.Here is an example from Sentry:![image](https://user-images.githubusercontent.com/2412177/101757346-572eda00-3ad7-11eb-9f57-6ba2b043594d.png)As you can see I get the error message, but I have to look through all my messages (in different bundles etc) to find the sinner.This PR adds the message name directly to the exception message.Commits-------d985ca9 [Messenger] Added more descriptive exception message when handling of a message failed
* 5.2:  Fix CS in Changelogs in 5.2
This PR was merged into the 5.3-dev branch.Discussion----------Fix CS in changelogs| Q             | A| ------------- | ---| Branch?       | 5.x| Bug fix?      | no| New feature?  | no| Deprecations? | no| Tickets       | -| License       | MIT| Doc PR        | -the last one :)Commits-------fb22eec Fix CS in changelogs
* 5.2:  [PropertyInfo][Serializer] Fixed extracting ignored properties  [travis] fix checking if the current branch has same major as the next release
* 5.2:  Cleanup composer.json files  [Notifier] Add PHP 8 support for bridges
@carsonbot
Copy link

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

Copy link
Member

@jderussejderusse left a comment

Choose a reason for hiding this comment

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

Indeed theTypes::DATE_IMMUTABLE case is handled at line 71

Could you please rebase on 5.2, and add a test to convert this case?

derrabus reacted with thumbs up emoji
@carsonbotcarsonbot changed the titleFix bug with date immutable and datetime immutable[DoctrineBridge] Fix bug with date immutable and datetime immutableDec 11, 2020
@jderussejderusse added this to the5.2 milestoneDec 11, 2020
@FlorianM11FlorianM11 changed the base branch from5.x to5.2December 14, 2020 16:20
derrabus added a commit that referenced this pull requestDec 16, 2020
…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
Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment

Reviewers

@jderussejderussejderusse left review comments

@chalasrchalasrAwaiting requested review from chalasrchalasr is a code owner

@srozesrozeAwaiting requested review from sroze

@wouterjwouterjAwaiting requested review from wouterjwouterj is a code owner

@xabbuhxabbuhAwaiting requested review from xabbuhxabbuh is a code owner

@ycerutoycerutoAwaiting requested review from ycerutoyceruto is a code owner

Assignees

No one assigned

Projects

None yet

Milestone

5.2

Development

Successfully merging this pull request may close these issues.

Bug with date immutable : hours, minutes and seconds has display

31 participants

@FlorianM11@carsonbot@jderusse@derrabus@nicolas-grekas@fabpot@Nyholm@chalasr@alexander-schranz@xabbuh@jacekwilczynski@tyx@karlshea@ogizanagi@TheGarious@simonberger@norkunas@malteschlueter@vudaltsov@CesarScur@lyrixx@TimoBakx@ronnylt@fbourigault@renan@thiagomp@awoimbee@VincentLanglet@ferrastas@loevgaard@guillaume-sainthillier

[8]ページ先頭

©2009-2025 Movatter.jp