Skip to content

bpo-38536: Removes trailing space in formatted currency#16864

Merged
methane merged 3 commits into
masterfrom
unknown repository
Jan 20, 2020
Merged

bpo-38536: Removes trailing space in formatted currency#16864
methane merged 3 commits into
masterfrom
unknown repository

Conversation

@ghost

@ghost ghost commented Oct 20, 2019 •
edited by bedevere-bot
Loading

Copy link
Copy Markdown

with international=True and symbol following value.

Validated that Ubuntu was affected
and Windows 10 was not affected.
Fix considers both cases.

https://bugs.python.org/issue38536

@the-knights-who-say-ni

the-knights-who-say-ni commented Oct 20, 2019

Copy link
Copy Markdown

Hello, and thanks for your contribution!

I'm a bot set up to make sure that the project can legally accept this contribution by verifying everyone involved has signed the PSF contributor agreement (CLA).

Recognized GitHub username

We couldn't find a bugs.python.org (b.p.o) account corresponding to the following GitHub usernames:

@sanjo, @harsh04081997

This might be simply due to a missing "GitHub Name" entry in one's b.p.o account settings. This is necessary for legal reasons before we can look at this contribution. Please follow the steps outlined in the CPython devguide to rectify this issue.

You can check yourself to see if the CLA has been received.

Thanks again for the contribution, we look forward to reviewing it!

eamanu
eamanu reviewed Oct 20, 2019
View reviewed changes
Comment thread Lib/locale.py Outdated

@eamanu eamanu Oct 20, 2019

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

why don't use rstrip()?

@ghost ghost Oct 21, 2019

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

To be more specific about the case (1 trailing space).

@eamanu

eamanu commented Oct 20, 2019

Copy link
Copy Markdown
Contributor

@sanjo Don't forget sign cla

@ghost

ghost commented Oct 21, 2019

Copy link
Copy Markdown
Author

CLA has been signed and https://check-python-cla.herokuapp.com/ also gave back success.

@Mariatta

Mariatta commented Oct 21, 2019

Copy link
Copy Markdown
Member

Thanks for fixing this bug. Can you please write the news entry.

@ghost

ghost commented Oct 21, 2019

Copy link
Copy Markdown
Author

Can you please write the news entry.

@Mariatta Done (with 17867bd).

eamanu
eamanu approved these changes Oct 22, 2019
View reviewed changes

@eamanu eamanu left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I don't know if write the example is a good improve. but LGTM :-)

@ghost

ghost commented Oct 31, 2019

Copy link
Copy Markdown
Author

Just FYI, the CLA has been signed by me. The bot seems to have some quirk with removing the label. Let me know if there is anything else necessary from my side for this pull request to be merged.

@Mariatta

Mariatta commented Nov 1, 2019

Copy link
Copy Markdown
Member

We check CLA record for everyone who made commit to the PR, not just the person who opened the PR. The code seems to be committed by user "A A", and perhaps "A A" has not signed CLA.

Either have A A sign the CLA (and create bpo account) or modify this PR so it only contains your own commit.

SanjoSolutions and others added 2 commits November 1, 2019 15:23
@SanjoSolutions
bpo-38536: Removes trailing space in formatted currency with internat…
e99bcc5 
…ional=True and symbol following value

Validated that Ubuntu was affected
and Windows 10 was not affected.
Fix considers both cases.
@blurb-it @SanjoSolutions
📜🤖 Added by blurb_it.
7f3525e
@ghost

ghost commented Nov 1, 2019

Copy link
Copy Markdown
Author

@Mariatta I have fixed the commit to contain the proper author and committer. There was just some configuration for Git missing on my newly setup computer. Best regards.

@methane

methane commented Nov 25, 2019

Copy link
Copy Markdown
Member

Validated that Ubuntu was affected
and Windows 10 was not affected.

Have you tried all locale on Ubuntu and Windows?
What is output of the Windows 10?

@csabella

csabella commented Jan 17, 2020

Copy link
Copy Markdown
Contributor

@sanjo, please address @methane's questions. Thank you!

@ghost

ghost commented Jan 17, 2020

Copy link
Copy Markdown
Author

Have you tried all locale on Ubuntu and Windows?

No, I have not tried the all locale on Ubuntu and Windows.

What is output of the Windows 10?

The output on Windows 10 has no trailing space.

methane
methane reviewed Jan 18, 2020
View reviewed changes
Comment thread Misc/NEWS.d/next/Library/2019-10-21-20-24-51.bpo-38536.beZ0Sk.rst Outdated
@sanjo @methane
Update Misc/NEWS.d/next/Library/2019-10-21-20-24-51.bpo-38536.beZ0Sk.rst
ffcc468 
Co-Authored-By: Inada Naoki <songofacandy@gmail.com>
@methane methane merged commit e96d954 into python:master Jan 20, 2020
shihai1991 pushed a commit to shihai1991/cpython that referenced this pull request Jan 31, 2020
@methane @shihai1991
bpo-38536: locale: Remove trailing space in formatted currency (pytho…
91a0674 
…nGH-16864)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

8 participants

@the-knights-who-say-ni @eamanu @Mariatta @methane @csabella @bedevere-bot @SanjoSolutions @sanjo