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

change targetEntity format#10696

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
osavchenko wants to merge1 commit intosymfony:masterfromosavchenko:changes_according_to_doctrine2
Closed

change targetEntity format#10696

osavchenko wants to merge1 commit intosymfony:masterfromosavchenko:changes_according_to_doctrine2

Conversation

@osavchenko
Copy link
Contributor

According to#9401 improved support for entity namespace aliases

/**
* @ORM\OneToMany(
* targetEntity="Comment",
* targetEntity="App\Entity\Comment",
Copy link
Contributor

Choose a reason for hiding this comment

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

I think it is better to suggestComment::class because if you refactor namespace it will be picked up and you don't have to edit a bunch of strings

Copy link
ContributorAuthor

Choose a reason for hiding this comment

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

The similar approach was applied before, so I decide to follow it. Do you want to suggest rework everywhere toClassNmae::class?

Copy link
Contributor

Choose a reason for hiding this comment

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

Ah, then leave it like this in PR. I think reworking it everywhere would be a nice addition. Not sure what docs team thinks about it

Copy link
ContributorAuthor

Choose a reason for hiding this comment

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

If team decide to rework everything, so, I'm in ;)

Copy link
Member

Choose a reason for hiding this comment

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

Using the class constant doesn't work in annotations if I am not completely mistaken.

Copy link
Contributor

Choose a reason for hiding this comment

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

It works, I used it like:

     * @ORM\ManyToOne(targetEntity=Comment::class)     * @ORM\JoinColumn(name="id", referencedColumnName="id")

When you refactor and maybe change the namespace, it changes the use statements (not sure about strings but it didn't work for me most of the times) so that is an advantage on usingComment::class instead of"App\Entity\Comment".

Copy link
Contributor

@kunicmarko20kunicmarko20Nov 25, 2018
edited
Loading

Choose a reason for hiding this comment

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

After discussing this with@ostrolucky on slack, it seems that:

clicking onmappedBy="foo"/inversedBy="foo"

dosen't work withComment::class also if you rename the class it still staysComment::class so I guess FQCN is still better

Copy link
ContributorAuthor

Choose a reason for hiding this comment

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

You mean "clicking on" IDE?

Copy link
Contributor

Choose a reason for hiding this comment

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

ye, clicking on the name of the var in your IDE (phpstorm in my case).

Copy link
Contributor

@kunicmarko20kunicmarko20 left a comment

Choose a reason for hiding this comment

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

Thank you for your help and congrats on your first contribution!

@osavchenko
Copy link
ContributorAuthor

osavchenko commentedNov 25, 2018 via email

Alright, thank uppity for explanation. So, this issue is resolved?нд, 25 лист. 2018, 18:01 Marko Kunic користувач notifications@github.comпише:
***@***.**** commented on this pull request. ------------------------------ In best_practices/business-logic.rst <#10696 (comment)> : > @@ -198,7 +198,7 @@ looking for mapping information:: /** *@Orm\OneToMany( - * targetEntity="Comment", + * targetEntity="App\Entity\Comment", ye, clicking on the name of the var in your IDE (phpstorm in my case). — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub <#10696 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAlqKnj8rdxXzw2qtSzcYAUu34ISXc59ks5uyr7tgaJpZM4Yv8bL> .

@xabbuhxabbuh added this to the2.8 milestoneNov 26, 2018
@javiereguiluz
Copy link
Member

Oleksandr thanks a lot for fixing this. It was a great first contribution to Symfony Docs!

javiereguiluz added a commit that referenced this pull requestNov 27, 2018
This PR was submitted for the master branch but it was merged into the 2.8 branch instead (closes#10696).Discussion----------change targetEntity formatAccording to#9401 improved support for entity namespace aliasesCommits-------1633ff0 change targetEntity format
@osavchenko
Copy link
ContributorAuthor

Thank you,@javiereguiluz! And thanks a lot for reviewers for hints.

@osavchenkoosavchenko deleted the changes_according_to_doctrine2 branchMarch 31, 2020 11:24
Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment

Reviewers

@xabbuhxabbuhxabbuh approved these changes

+2 more reviewers

@maxheliasmaxheliasmaxhelias approved these changes

@kunicmarko20kunicmarko20kunicmarko20 approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Projects

None yet

Milestone

2.8

Development

Successfully merging this pull request may close these issues.

6 participants

@osavchenko@javiereguiluz@xabbuh@maxhelias@kunicmarko20@carsonbot

[8]ページ先頭

©2009-2025 Movatter.jp