Skip to content

Allow TranslatableMessage object in form option 'help' #41717

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.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
merged 1 commit into from
Jul 4, 2021
Merged

Allow TranslatableMessage object in form option 'help' #41717

merged 1 commit into from
Jul 4, 2021

Conversation

plandolt
Copy link
Contributor

Q A
Branch? 5.4
Bug fix? no
New feature? yes
Deprecations? no
Tickets n/a
License MIT
Doc PR -

This PR allows the use of TranslatableMessage (or t(...)) for a help option the same way the label option already supports this.

@carsonbot
Copy link

Hey!

To help keep things organized, we don't allow "Draft" pull requests. Could you please click the "ready for review" button or close this PR and open a new one when you are done?

Note that a pull request does not have to be "perfect" or "ready for merge" when you first open it. We just want it to be ready for a first review.

Cheers!

Carsonbot

@plandolt plandolt marked this pull request as ready for review June 15, 2021 15:28
@plandolt plandolt requested review from xabbuh and yceruto as code owners June 15, 2021 15:28
@nicolas-grekas nicolas-grekas added this to the 5.4 milestone Jun 17, 2021
@plandolt plandolt requested a review from OskarStark June 18, 2021 06:30
@OskarStark
Copy link
Contributor

Can you add a test case that this works?

@plandolt plandolt requested a review from xabbuh June 18, 2021 12:49
xabbuh
xabbuh approved these changes Jun 18, 2021
View reviewed changes
Copy link
Member

@xabbuh xabbuh left a comment

Choose a reason for hiding this comment

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

with a minor comment

@plandolt
Copy link
Contributor Author

updated the changelog message according to @xabbuh his comment.

BTW: Is it better for you to leave all the commits or squash them regularly and force push this PR?

@xabbuh
Copy link
Member

xabbuh commented Jun 21, 2021

@scuben Squashing and rebasing is fine, but when can also squash commits while merging as long the PR doesn't have conflicts with the target branch (otherwise it will need to be rebased).

@fabpot
Copy link
Member

fabpot commented Jul 4, 2021

Thank you @scuben.

@fabpot fabpot merged commit 0e133be into symfony:5.4 Jul 4, 2021
@fabpot fabpot mentioned this pull request Jul 4, 2021
Closed
@plandolt plandolt deleted the patch-2 branch July 4, 2021 12:06
@plandolt plandolt mentioned this pull request Jul 4, 2021
Closed
@Tobion Tobion mentioned this pull request Jul 13, 2021
Open
This was referenced Nov 5, 2021
Merged
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@fabpot fabpot fabpot approved these changes

@javiereguiluz javiereguiluz javiereguiluz approved these changes

@xabbuh xabbuh xabbuh approved these changes

@yceruto yceruto yceruto approved these changes

@OskarStark OskarStark Awaiting requested review from OskarStark

Assignees
No one assigned
Labels
Feature Status: Reviewed
Projects
None yet
Milestone
5.4
Development

Successfully merging this pull request may close these issues.
8 participants
@plandolt @carsonbot @OskarStark @xabbuh @fabpot @javiereguiluz @yceruto @nicolas-grekas