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

[Form] ability to set rounding strategy for MoneyType#26767

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
fabpot merged 1 commit intosymfony:masterfromsyastrebov:set-money-rounding-mode
Apr 4, 2018

Conversation

@syastrebov
Copy link
Contributor

QA
Branch?master
Bug fix?no
New feature?yes
BC breaks?no
Deprecations?no
Tests pass?yes
Fixed ticketsno
LicenseMIT
Doc PRsymfony/symfony-docs#9543

Addedrounding_mode to theMoneyType to be possible to change rounding strategy for money values. For now it's justROUND_HALF_UP but it's good to haveROUND_DOWN as well. E.g. to transform15.999 to15.99 instead of15.1.

ro0NL and erlangparasu reacted with thumbs up emoji
@syastrebovsyastrebov changed the titleability to set rounding strategyability to set rounding strategy for MoneyTypeApr 3, 2018
@nicolas-grekasnicolas-grekas added this to the4.1 milestoneApr 3, 2018
@nicolas-grekas
Copy link
Member

nicolas-grekas commentedApr 3, 2018
edited
Loading

would you mind adding some tests please?
(and congrats and thank you for your first PR :) )

@syastrebovsyastrebov changed the titleability to set rounding strategy for MoneyType[Form] ability to set rounding strategy for MoneyTypeApr 3, 2018
@syastrebovsyastrebovforce-pushed theset-money-rounding-mode branch frome63a499 tof3b1424CompareApril 4, 2018 11:15
@syastrebov
Copy link
ContributorAuthor

@nicolas-grekas You're welcome :) Added tests for the changes, please take a look.

@fabpot
Copy link
Member

Thank you@syastrebov.

@fabpotfabpot merged commitf3b1424 intosymfony:masterApr 4, 2018
fabpot added a commit that referenced this pull requestApr 4, 2018
…(syastrebov)This PR was merged into the 4.1-dev branch.Discussion----------[Form] ability to set rounding strategy for MoneyType| Q             | A| ------------- | ---| Branch?       | master| Bug fix?      | no| New feature?  | yes| BC breaks?    | no| Deprecations? | no| Tests pass?   | yes| Fixed tickets | no| License       | MIT| Doc PR        |symfony/symfony-docs#9543Added `rounding_mode` to the `MoneyType` to be possible to change rounding strategy for money values. For now it's just `ROUND_HALF_UP` but it's good to have `ROUND_DOWN` as well. E.g. to transform `15.999` to `15.99` instead of `15.1`.Commits-------f3b1424 rounding_mode for money type
@syastrebovsyastrebov deleted the set-money-rounding-mode branchApril 5, 2018 08:20
javiereguiluz added a commit to symfony/symfony-docs that referenced this pull requestApr 16, 2018
This PR was merged into the master branch.Discussion----------rounding_mode for money typeAdded docs forsymfony/symfony#26767Commits-------819d0a6 rounding_mode for money type
@fabpotfabpot mentioned this pull requestMay 7, 2018
));

$resolver->setAllowedValues('rounding_mode',array(
NumberToLocalizedStringTransformer::ROUND_FLOOR,
Copy link
Contributor

Choose a reason for hiding this comment

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

Why usingNumberToLocalizedStringTransformer and notNumberFormatter here ? Adding a dependency to a transformer without using it doesn't seem right.

Copy link
Member

Choose a reason for hiding this comment

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

NumberToLocalizedStringTransformer is the parent class of theMoneyToLocalizedStringTransformer which is used by this form type and that is configured through these options.

Copy link
Contributor

Choose a reason for hiding this comment

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

Then why notMoneyToLocalizedStringTransformer::ROUND_FLOOR ?

@erlangparasu
Copy link

@syastrebov how to round-up by 100, sir? round-down by 100?

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

Reviewers

@xabbuhxabbuhxabbuh left review comments

@fabpotfabpotfabpot approved these changes

@nicolas-grekasnicolas-grekasnicolas-grekas approved these changes

+2 more reviewers

@ValouleloupValouleloupValouleloup left review comments

@ro0NLro0NLro0NL approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Projects

None yet

Milestone

4.1

Development

Successfully merging this pull request may close these issues.

8 participants

@syastrebov@nicolas-grekas@fabpot@erlangparasu@ro0NL@xabbuh@Valouleloup@carsonbot

[8]ページ先頭

©2009-2025 Movatter.jp