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

[OptionResolver] Document the OptionConfigurator#12426

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

lmillucci
Copy link
Contributor

Adddefine() method to OptionResolver to enhance readability of option configuration. Seesymfony/symfony#33848

@OskarStarkOskarStark added OptionsResolver Waiting Code MergeDocs for features pending to be merged labelsOct 5, 2019
@OskarStarkOskarStark added this to the4.4 milestoneOct 5, 2019
nicolas-grekas added a commit to symfony/symfony that referenced this pull requestFeb 6, 2020
…define options with fluent interface (lmillucci, yceruto)This PR was merged into the 5.1-dev branch.Discussion----------[OptionsResolver] Add new OptionConfigurator class to define options with fluent interface| Q             | A| ------------- | ---| Branch?       | master| Bug fix?      | no| New feature?  | yes| Deprecations? | no| Tickets       |Fix#33735| License       | MIT| Doc PR        |symfony/symfony-docs#12426- [x] submit changes to the documentationThis PR adds OptionConfigurator to the OptionsResolverCommits-------1ff5640 [OptionsResolver] Add new OptionConfigurator class to define options with fluent interface
@nicolas-grekasnicolas-grekas modified the milestones:4.4,5.1Feb 6, 2020
@OskarStarkOskarStark removed the Waiting Code MergeDocs for features pending to be merged labelFeb 7, 2020
@wouterjwouterj changed the title[WCM][OptionResolver] document OptionConfigurator (fix #33848)[OptionResolver] document OptionConfigurator (fix #33848)Feb 9, 2020
Copy link
Member

@wouterjwouterj left a comment

Choose a reason for hiding this comment

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

Hi@lmillucci!

This is a great new syntax! Is there any drawback to using this new signature? I'm asking because if there is no drawback (performance wise or the like), we should imho use this new signature everywhere in this document. A superior syntax should be preferred over the old setters :)

If you agree, but don't have the time/motivation/whatever to do it, please leave a comment and I'll take care for it while merging this PR.

Thanks!

yceruto reacted with thumbs up emoji
@lmillucci
Copy link
ContributorAuthor

lmillucci commentedFeb 9, 2020 via email

I don't think there are any drawback in this signature.I don't think I will have time to work on this until next weekend so if youwant to do this in the meantime you can work on it :)Il dom 9 feb 2020, 16:43 Wouter J <notifications@github.com> ha scritto:
…
***@***.**** approved this pull request. Hi@lmillucci <https://github.com/lmillucci>! This is a great new syntax! Is there any drawback to using this new signature? I'm asking because if there is no drawback (performance wise or the like), we should imho use this new signature everywhere in this document. A superior syntax should be preferred over the old setters :) If you agree, but don't have the time/motivation/whatever to do it, please leave a comment and I'll take care for it while merging this PR. Thanks! — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#12426?email_source=notifications&email_token=ABLXDXAZ4QZDXQFFXFAAUI3RCAQAPA5CNFSM4I5XAG2KYY3PNVWWK3TUL52HS4DFWFIHK3DMKJSXC5LFON2FEZLWNFSXPKTDN5WW2ZLOORPWSZGOCUZEE7Y#pullrequestreview-355615359>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/ABLXDXGF3LRCZSNPHHECGIDRCAQAPANCNFSM4I5XAG2A> .
wouterj and yceruto reacted with thumbs up emojiwouterj reacted with rocket emoji

@yceruto
Copy link
Member

Meanwhile, a newinfo() method was added to this new configurator. It would be nice if we could document it right here :)

OskarStark, wouterj, and rvanlaak reacted with thumbs up emoji

@yceruto
Copy link
Member

this is the link#13090

@OskarStarkOskarStark changed the title[OptionResolver] document OptionConfigurator (fix #33848)[OptionResolver] Document the OptionConfiguratorFeb 19, 2020
@OskarStarkOskarStarkforce-pushed thefix_33735_option_resolver branch from658a0f5 to921c73fCompareFebruary 19, 2020 07:14
@OskarStark
Copy link
Contributor

Thanks for your work on this new feature!

OskarStark added a commit that referenced this pull requestFeb 19, 2020
…lucci)This PR was squashed before being merged into the master branch.Discussion----------[OptionResolver] Document the OptionConfiguratorAdd `define()` method to OptionResolver to enhance readability of option configuration. Seesymfony/symfony#33848Commits-------921c73f [OptionResolver] Document the OptionConfigurator
@OskarStarkOskarStark merged commit921c73f intosymfony:masterFeb 19, 2020
OskarStark added a commit that referenced this pull requestFeb 19, 2020
@OskarStark
Copy link
Contributor

@yceruto to not get this PR rotten, I merged the current state 👍

Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment
Reviewers

@nicolas-grekasnicolas-grekasnicolas-grekas left review comments

@wouterjwouterjwouterj approved these changes

@ycerutoycerutoyceruto approved these changes

Assignees
No one assigned
Projects
None yet
Milestone
5.1
Development

Successfully merging this pull request may close these issues.

6 participants
@lmillucci@yceruto@OskarStark@nicolas-grekas@wouterj@carsonbot
[8]ページ先頭
©2009-2025 Movatter.jp