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

fix(common): remove trailing whitespace for CurrencyPipe #34642

Conversation

@troelslenda
Copy link
Contributor

troelslenda commented Jan 5, 2020

Trimming any surrounding whitespace characters in
formatNumberToLocaleString if currency symbol is
supressed.

Closes #34641

PR Checklist

Please check if your PR fulfills the following requirements:

PR Type

What kind of change does this PR introduce?

  •  Bugfix
  •  Feature
  •  Code style update (formatting, local variables)
  •  Refactoring (no functional changes, no api changes)
  •  Build related changes
  •  CI related changes
  •  Documentation content changes
  •  angular.io application / infrastructure changes
  •  Other... Please describe:

What is the current behavior?

Issue Number: #34641

What is the new behavior?

The exessive whitespace is trimmed.

Does this PR introduce a breaking change?

  •  Yes
  •  No
@troelslenda troelslenda requested review from angular/fw-core as code owners Jan 5, 2020
@googlebot googlebot added the cla: yes label Jan 5, 2020
Copy link
Member

petebacondarwin left a comment

Thanks for this fix @troelslenda - the main fix and the tests look great. Could you remove the additional refactorings that were added (i.e. the addition and removal of parentheses, and the reformatting) since they are not relevant to this fix.

@ngbot ngbot bot modified the milestone: needsTriage Jan 6, 2020
@troelslenda troelslenda force-pushed the troelslenda:remove-currency-pipe-trailing-space branch from 5812329 to f051afd Jan 6, 2020
@troelslenda

This comment has been minimized.

Copy link
Contributor Author

troelslenda commented Jan 6, 2020

Hi @petebacondarwin!

Thank you for your quick response. I've cleaned up the PR with a git commit --amend.

@petebacondarwin

This comment has been minimized.

Copy link
Member

petebacondarwin commented Jan 6, 2020

Great! Thanks.
Unfortunately, your amended commit message is all on one line, which is breaking the lint checks.

Trimming any surrounding whitespace characters informatNumberToLocaleString
if currency symbol issupressed.

Closes #34641
@troelslenda troelslenda force-pushed the troelslenda:remove-currency-pipe-trailing-space branch from f051afd to fb184e9 Jan 6, 2020
@troelslenda

This comment has been minimized.

Copy link
Contributor Author

troelslenda commented Jan 6, 2020

Hi @petebacondarwin
It should be good now. No failing tests it seems

@AndrewKushnir

This comment has been minimized.

Copy link
Contributor

AndrewKushnir commented Jan 6, 2020

alxhub added a commit that referenced this pull request Jan 7, 2020
Trimming any surrounding whitespace characters informatNumberToLocaleString
if currency symbol issupressed.

Closes #34641

PR Close #34642
@alxhub alxhub closed this in 1b6ad15 Jan 7, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants
You can’t perform that action at this time.