Skip to content
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 our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Form] add choice_translation_domain option to date types #6302

Merged

Conversation

HeahDude
Copy link
Contributor

Q A
Doc fix? yes
New docs? no
Applies to 2.8+
Fixed tickets partially #5934

@@ -0,0 +1,15 @@
choice_translation_domain
Copy link
Member

Choose a reason for hiding this comment

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

Shouldn't we reuse the already existing file from the choice type?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I would have. Unfortunately there is a conflict between added version : 2.7 for choice types but 2.8 for date types.
Do you know a way to override only the note in a block like this ?

Copy link
Member

Choose a reason for hiding this comment

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

Unfortunately no. But maybe we can update the description of the existing option with what you used here (imo it's more clear than what we already have).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I don't understand, I've just copy-pasted the current description and changed the version added.

@HeahDude
Copy link
Contributor Author

This one should be ready to be merged.

@xabbuh
Copy link
Member

xabbuh commented Apr 30, 2016

👍 I like it.

@wouterj wouterj merged commit 540cefe into symfony:2.8 May 5, 2016
wouterj added a commit that referenced this pull request May 5, 2016
…ypes (HeahDude)

This PR was merged into the 2.8 branch.

Discussion
----------

[Form] add `choice_translation_domain` option to date types

| Q             | A
| ------------- | ---
| Doc fix?      | yes
| New docs?     | no
| Applies to    | 2.8+
| Fixed tickets | partially #5934

Commits
-------

540cefe [Form] add `choice_translation_domain` option to date types
wouterj added a commit that referenced this pull request May 5, 2016
@wouterj
Copy link
Member

wouterj commented May 5, 2016

Thanks @HeahDude!

I've created another include file with the duplicated body and included that file in both choice_translation_domain files in 9448b1b . That solves the duplication problem.

@HeahDude HeahDude deleted the add-datetype-choice_translation_domain branch May 5, 2016 21:10
@HeahDude
Copy link
Contributor Author

HeahDude commented May 5, 2016

@wouterj thanks, great idea! So we can only use that file in 3.x getting rid of version added notes, right?

@wouterj
Copy link
Member

wouterj commented May 5, 2016

Yes, indeed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants