-
Notifications
You must be signed in to change notification settings - Fork 7.6k
Add -AsUTC to the Get-Date cmdlet #11611
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
Conversation
public SwitchParameter AsUTC | ||
{ | ||
get { return _asUTC; } | ||
|
||
set { _asUTC = value; } | ||
} | ||
|
||
private SwitchParameter _asUTC = false; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please don't use private fields.
public SwitchParameter AsUTC { get; set; }
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done.
(fwiw, I was trying to replicate how other fields in this Cmdlet are handled with public/private fields)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
fwiw, the Codacy PR tool disagrees with you :)
https://app.codacy.com/manual/TravisEz13/PowerShell_PowerShell/pullRequest?prid=4837266
Get-date -Date:"2020-01-01T00:00:00" -Format:"MMM-dd-yy hh:mm" | Should -Be "Jan-01-20 12:00" | ||
Get-date -Date:"2020-01-01T00:00:00" -Format:"MMM-dd-yy hh:mm" -AsUTC | Should -Be "Jan-01-20 08:00" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I guess the tests fail because CI VMs has already UTC time zone. In any case we should take into account time zone.
Perhaps follow will be more reliable test:
(Get-date -Date:"2020-01-01T00:00:00").Kind | Should -Be [DateTimeKind]::Local
(Get-date -Date:"2020-01-01T00:00:00" -AsUTC).Kind | Should -Be [DateTimeKind]::Utc
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done
(I had to modify the test a little from what you wrote, but I think it captures the same thing)
@brendandburns Please don't edit/trim "PR Checklist". |
Comments addressed, apologies for clipping the checklist, still learning the ropes around here :) Please take another look. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM with one comment
src/Microsoft.PowerShell.Commands.Utility/commands/utility/GetDateCommand.cs
Outdated
Show resolved
Hide resolved
@brendandburns Thank you! |
🎉 Handy links: |
PR Summary
Adds a
-AsUTC
flag to convert a date to UTC before formatting.PR Context
Requested in #11410
PR Checklist
.h
,.cpp
,.cs
,.ps1
and.psm1
files have the correct copyright header