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

[FrameworkBundle] MakeAbstractController::render() able to deal with forms and deprecaterenderForm()#46854

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

@nicolas-grekas
Copy link
Member

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

No need forAbstractController::renderForm() whenAbstractController::render() can do it!

yceruto and mdoutreluingne reacted with thumbs up emojiDavidBadura reacted with confused emoji
@HeahDude
Copy link
Contributor

With this change the API becomes implicit and too magical, 👎 from me.

DavidBadura reacted with thumbs up emoji

@fabpot
Copy link
Member

I suggested this change and I think it makes a lot of sense. The current method looks weird to me as it does not render a form ; it’s very confusing.

@HeahDude
Copy link
Contributor

I see the point, maybe better phpdoc blocks would help describing what these methods do?

Copy link
Contributor

@HeahDudeHeahDude left a comment

Choose a reason for hiding this comment

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

After discussing with Nicolas, I admit I was already -1 on the previous implementation, so it might be better this way indeed.

Copy link
Member

@chalasrchalasr left a comment

Choose a reason for hiding this comment

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

UPGRADE note missing :)

@nicolas-grekas
Copy link
MemberAuthor

UPGRADEd

HeahDude reacted with laugh emoji

HeahDude added a commit to HeahDude/symfony that referenced this pull requestSep 16, 2022
HeahDude added a commit to HeahDude/symfony that referenced this pull requestSep 17, 2022
HeahDude added a commit to HeahDude/symfony that referenced this pull requestSep 18, 2022
HeahDude added a commit to HeahDude/symfony that referenced this pull requestSep 18, 2022
@fabpotfabpot mentioned this pull requestOct 24, 2022
Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment

Reviewers

@chalasrchalasrchalasr requested changes

@dunglasdunglasdunglas approved these changes

@kbondkbondkbond approved these changes

@lyrixxlyrixxlyrixx approved these changes

+1 more reviewer

@HeahDudeHeahDudeHeahDude approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Projects

None yet

Milestone

6.2

Development

Successfully merging this pull request may close these issues.

8 participants

@nicolas-grekas@HeahDude@fabpot@dunglas@kbond@lyrixx@chalasr@carsonbot

[8]ページ先頭

©2009-2025 Movatter.jp