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

[Bug] .ToString("yyyy-MM-dd HH:mm:ss") is not "culture safe" and ":" can be converted to "." #6909

Closed
andreasjordan opened this issue Oct 6, 2020 · 5 comments · Fixed by #6911

Comments

@andreasjordan
Copy link
Contributor

I open this issue to discuss a general solution for converting datetimes into strings.

We found this here: #6452

I used VSCode and searched the code for "yyyy-MM-ddTHH:mm:ss" and found this in:

  • Get-DbaDbBackupHistory
  • Get-DbaDbMailHistory
  • Get-DbaDbMailLog
  • Get-DbaDbRestoreHistory

I used VSCode and searched the code for "yyyy-MM-dd HH:mm:ss" and found this in:

  • Export-DbaExecutionPlan
  • Get-DbaExecutionPlan
  • Get-DbaRandomizedValue
  • Invoke-DbaDbDataMasking

@niphlod came up with .ToString("yyyy-MM-ddTHH:mm:ss", [System.Globalization.CultureInfo]::InvariantCulture).

Are there other ideas?

@andreasjordan
Copy link
Contributor Author

@niphlod niphlod changed the title [Bug] .ToString("yyyy-MM-dd HH:mm:ss") is not "culture save" and ":" can be converted to "." [Bug] .ToString("yyyy-MM-dd HH:mm:ss") is not "culture safe" and ":" can be converted to "." Oct 7, 2020
@niphlod
Copy link
Contributor

niphlod commented Oct 7, 2020

The uninternationally-savvy kids set the culture on the entire process and live with it, but we chose not to force it ..... wherever we do a tostring specifying the culture we want (or, the invariantculture for better) is the only way.

@potatoqualitee
Copy link
Member

If the change is agreed upon (i'm not savvy enough in this arena), please create 1 PR for all commands. I would like @niphlod's approval because of his background. @FriedrichWeinmann is also an ideal candidate.

@niphlod
Copy link
Contributor

niphlod commented Oct 8, 2020

we already switched from 120 to 126 (the "T" made the difference) for some other "culture issue", and we thought we fixed for good.
Surely this doesn't break anything. If anything else more comes up, we'll fix it down the line.
I just have a small doubt about [System.Globalization.CultureInfo]::InvariantCulture not being compatible with PS3, but I'm fairly confident it works.
so +1 for me.

@andreasjordan
Copy link
Contributor Author

I will try to find time for this this afternoon. If someone else wants to take over, please let me know.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants