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.

Sign up for GitHub

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

Jump to bottom

CurrencyNegativePattern can be 16 #9081

Merged
merged 9 commits into from
Jun 16, 2023
Merged

CurrencyNegativePattern can be 16 #9081

merged 9 commits into from
Jun 16, 2023

Conversation

AlexRadch
Copy link
Contributor

Summary

CurrencyNegativePattern can return a value greater than 15. See code.

Fixes dotnet/runtime#87643

@AlexRadch AlexRadch requested a review from a team as a code owner June 16, 2023 05:36
@ghost ghost added the community-contribution Indicates that the PR has been added by a community member label Jun 16, 2023
@ghost
Copy link

ghost commented Jun 16, 2023

Tagging subscribers to this area: @dotnet/area-system-globalization
See info in area-owners.md if you want to be subscribed.

Issue Details

Summary

CurrencyNegativePattern can return a value greater than 15. See code.

Fixes dotnet/runtime#87643

Author: AlexRadch
Assignees: -
Labels:

area-System.Globalization, community-contribution
Milestone: -

@AlexRadch AlexRadch mentioned this pull request Jun 16, 2023
Closed
@learn-build-service-prod

This comment was marked as outdated.

Sign in to view
@adamsitnik adamsitnik requested a review from tarekgh June 16, 2023 06:57
gewarren
gewarren reviewed Jun 16, 2023
View reviewed changes
Copy link
Contributor

@gewarren gewarren left a comment

Choose a reason for hiding this comment

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

Thanks @AlexRadch! Could you also add a simple VB project file so our automation can compile that code?

xml/System.Globalization/NumberFormatInfo.xml Outdated Show resolved Hide resolved
tarekgh
tarekgh approved these changes Jun 16, 2023
View reviewed changes
Copy link
Member

@tarekgh tarekgh left a comment

Choose a reason for hiding this comment

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

Modulo @gewarren comments and my suggestion, LGTM.

@AlexRadch
Copy link
Contributor Author

Thanks @AlexRadch! Could you also add a simple VB project file so our automation can compile that code?

@gewarren Can you give me an example? I will.

@gewarren
Copy link
Contributor

@AlexRadch Something like this, but you can target .NET 7.

https://github.com/dotnet/dotnet-api-docs/blob/main/snippets/visualbasic/System.Net.Security/ALPN/Project.vbproj

@learn-build-service-prod

This comment was marked as outdated.

Sign in to view
@learn-build-service-prod
Copy link

Learn Build status updates of commit 9dcf116:

✅ Validation status: passed

File Status Preview URL Details
snippets/csharp/System.Globalization/NumberFormatInfo/CurrencyNegativePattern/currencynegativepattern1.cs ✅Succeeded View
snippets/csharp/System.IdentityModel.Tokens/IssuerNameRegistry/Overview/scripts/microsoftajax.debug.js ✅Succeeded
snippets/visualbasic/VS_Snippets_CLR_System/system.globalization.numberformatinfo.currencynegativepattern/vb/currencynegativepattern1.vb ✅Succeeded View
xml/System.Globalization/NumberFormatInfo.xml ✅Succeeded View

For more details, please refer to the build report.

For any questions, please:

gewarren
gewarren approved these changes Jun 16, 2023
View reviewed changes
Copy link
Contributor

@gewarren gewarren left a comment

Choose a reason for hiding this comment

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

A few final suggestions that I'll commit.

...rp/System.Globalization/NumberFormatInfo/CurrencyNegativePattern/currencynegativepattern1.cs Outdated Show resolved Hide resolved
...rp/System.Globalization/NumberFormatInfo/CurrencyNegativePattern/currencynegativepattern1.cs Outdated Show resolved Hide resolved
...system.globalization.numberformatinfo.currencynegativepattern/vb/currencynegativepattern1.vb Outdated Show resolved Hide resolved
...system.globalization.numberformatinfo.currencynegativepattern/vb/currencynegativepattern1.vb Outdated Show resolved Hide resolved
@learn-build-service-prod
Copy link

Learn Build status updates of commit 6e7e544:

✅ Validation status: passed

File Status Preview URL Details
snippets/csharp/System.Globalization/NumberFormatInfo/CurrencyNegativePattern/currencynegativepattern1.cs ✅Succeeded View
snippets/csharp/System.IdentityModel.Tokens/IssuerNameRegistry/Overview/scripts/microsoftajax.debug.js ✅Succeeded
snippets/visualbasic/VS_Snippets_CLR_System/system.globalization.numberformatinfo.currencynegativepattern/vb/currencynegativepattern1.vb ✅Succeeded View
snippets/visualbasic/VS_Snippets_CLR_System/system.globalization.numberformatinfo.currencynegativepattern/vb/Project.vbproj ✅Succeeded
xml/System.Globalization/NumberFormatInfo.xml ✅Succeeded View

For more details, please refer to the build report.

For any questions, please:

@gewarren gewarren merged commit d617861 into dotnet:main Jun 16, 2023
@AlexRadch AlexRadch deleted the CurrencyNegativePattern_16 branch June 18, 2023 16:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@tarekgh tarekgh tarekgh approved these changes

@gewarren gewarren gewarren approved these changes

Assignees
No one assigned
Labels
area-System.Globalization community-contribution Indicates that the PR has been added by a community member
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

CurrencyNegativePattern can be 16
3 participants
@AlexRadch @gewarren @tarekgh