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

[Serializer] Fallback looking for DiscriminatorMap on interfaces#47221

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:5.4fromyurplan:serializer-multiple-interfaces
Sep 10, 2023

Conversation

@Caligone
Copy link
Contributor

@CaligoneCaligone commentedAug 8, 2022
edited
Loading

QA
Branch?6.2
Bug fix?yes
New feature?no
Deprecations?no
TicketsFix#47219
LicenseMIT
Doc PRNo

First time here
Trying to properlyfix#47219 but I'm not sure about the impact of this changes
I am also not sure either if it should be a 6.2 or 6.1 based branch

  • Globally validate the behavior
  • Add tests

David-Moisan reacted with thumbs up emoji
@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 6.2 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

@carsonbotcarsonbot added this to the6.2 milestoneAug 8, 2022
@carsonbot
Copy link

It looks like you unchecked the "Allow edits from maintainer" box. That is fine, but please note that if you have multiple commits, you'll need to squash your commits into one before this can be merged. Or, you can check the "Allow edits from maintainers" box and the maintainer can squash for you.

Cheers!

Carsonbot

@CaligoneCaligone changed the title[Serializer] Fallback Looking for DiscriminatorMap on interfaces[Serializer] Fallback looking for DiscriminatorMap on interfacesAug 8, 2022
@CaligoneCaligone marked this pull request as draftAugust 8, 2022 12:10
Caligone added a commit to yurplan/serializer that referenced this pull requestAug 9, 2022
@nicolas-grekasnicolas-grekas modified the milestones:6.2,6.3Nov 5, 2022
@CaligoneCaligone marked this pull request as ready for reviewNovember 24, 2022 09:30
@carsonbotcarsonbot modified the milestones:6.3,6.2Nov 24, 2022
@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 6.2 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

@carsonbot
Copy link

It looks like you unchecked the "Allow edits from maintainer" box. That is fine, but please note that if you have multiple commits, you'll need to squash your commits into one before this can be merged. Or, you can check the "Allow edits from maintainers" box and the maintainer can squash for you.

Cheers!

Carsonbot

@derrabus
Copy link
Member

Hello. Sorry nobody has picked up your PR yet. I have to ask you to provide a test for your change. This helps a reviewer understand the problem that you're attempting to fix. The test also makes sure, your bug is not re-introduced by accident.

@CaligoneCaligoneforce-pushed theserializer-multiple-interfaces branch from83bd108 toed8336fCompareNovember 29, 2022 08:01
@Caligone
Copy link
ContributorAuthor

No worries
As requested, I added a test
I would be happy to receive feedback and make adjustments as needed

@CaligoneCaligoneforce-pushed theserializer-multiple-interfaces branch fromed8336f to5333c6dCompareMay 11, 2023 08:14
@nicolas-grekasnicolas-grekas modified the milestones:6.3,6.4May 23, 2023
Copy link
Contributor

@mtarldmtarld left a comment

Choose a reason for hiding this comment

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

Almost good!

@mtarld
Copy link
Contributor

As it's an old bug, could you target the5.4 branch instead?

@CaligoneCaligoneforce-pushed theserializer-multiple-interfaces branch from5333c6d tof282ca2CompareSeptember 2, 2023 07:42
@CaligoneCaligone changed the base branch from6.4 to5.4September 2, 2023 07:43
@CaligoneCaligoneforce-pushed theserializer-multiple-interfaces branch fromf282ca2 tobfc4bddCompareSeptember 2, 2023 07:46
@Caligone
Copy link
ContributorAuthor

Thanks for the feedback !
I rebased and fixed last issues

@fabpotfabpot modified the milestones:6.4,5.4Sep 10, 2023
@fabpot
Copy link
Member

Thank you@Caligone.

Caligone reacted with rocket emoji

@fabpotfabpot merged commit9271349 intosymfony:5.4Sep 10, 2023
This was referencedSep 30, 2023
Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment

Reviewers

@stofstofstof left review comments

@fabpotfabpotfabpot approved these changes

@mtarldmtarldmtarld approved these changes

@dunglasdunglasAwaiting requested review from dunglasdunglas is a code owner

Assignees

No one assigned

Projects

None yet

Milestone

5.4

Development

Successfully merging this pull request may close these issues.

[Serializer] DiscriminatorMap with multiple interfaces/abstract class

7 participants

@Caligone@carsonbot@derrabus@mtarld@fabpot@stof@nicolas-grekas

[8]ページ先頭

©2009-2025 Movatter.jp