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

Use InvariantCulture for ChangeType() and decimal.TryParse() #35675

Open
wants to merge 7 commits into
base: main
Choose a base branch
from

Conversation

ErikEJ
Copy link
Contributor

@ErikEJ ErikEJ commented Feb 24, 2025

fixes #35654

  • I've read the guidelines for contributing and seen the walkthrough
  • I've posted a comment on an issue with a detailed description of how I am planning to contribute and got approval from a member of the team
  • The code builds and tests pass locally (also verified by our automated build checks)
  • Tests for the changes have been added (for bug fixes / features)
  • Code follows the same patterns and style as existing code in this repo

@ErikEJ ErikEJ requested a review from a team as a code owner February 24, 2025 10:14
@roji
Copy link
Member

roji commented Feb 24, 2025

@ErikEJ seems like we have the same bug in the SQLite provider, can you please make the same change there?

Add "international" tests
@ErikEJ
Copy link
Contributor Author

ErikEJ commented Feb 24, 2025

@roji PR updated

@roji
Copy link
Member

roji commented Feb 25, 2025

@ErikEJ FYI there are build failures.

@ErikEJ
Copy link
Contributor Author

ErikEJ commented Feb 25, 2025

@roji Getting some interesting SQLite errors now, how can I debug the tests in Visual Studio? (I do not have "VS 17.16")

@roji
Copy link
Member

roji commented Feb 26, 2025

I think new/preview VS is probably needed (because of the preview dotnet SDK that's required). If for some reason you can't/don't want to install that, you can work on the 9.0 branch for now (for debugging) and once everything works there, update the PR against 10...

@ErikEJ
Copy link
Contributor Author

ErikEJ commented Feb 26, 2025

I am using preview, but did not have .NET 10 (is is brand new) - I will repro in the 9.0 branch, and port to main.

@ErikEJ ErikEJ changed the title Use Invariant Culture for ChangeType Use InvariantCulture for ChangeType() and decimal.TryParse() Feb 26, 2025
@ErikEJ
Copy link
Contributor Author

ErikEJ commented Feb 26, 2025

@roji Found another bug - could be "interesting" to run the entire test suite with "da-DK" 😆

@ErikEJ
Copy link
Contributor Author

ErikEJ commented Feb 26, 2025

@roji Finally green 🎉 😆

@ErikEJ ErikEJ requested a review from roji February 26, 2025 12:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Scaffold uses system's comma separator for HasDefaultValue
3 participants