Add -UnixTimeSeconds to Get-Date to allow Unix time input#13084
Add -UnixTimeSeconds to Get-Date to allow Unix time input#13084daxian-dbw merged 19 commits intoPowerShell:masterfrom aetos382:add-unixtime-to-get-date
-UnixTimeSeconds to Get-Date to allow Unix time input#13084Conversation
src/Microsoft.PowerShell.Commands.Utility/commands/utility/GetDateCommand.cs
Outdated
Show resolved
Hide resolved
src/Microsoft.PowerShell.Commands.Utility/commands/utility/GetDateCommand.cs
Outdated
Show resolved
Hide resolved
|
@jackdcasey @mklement0 Please review. |
src/Microsoft.PowerShell.Commands.Utility/commands/utility/GetDateCommand.cs
Outdated
Show resolved
Hide resolved
|
@TravisEz13 and @iSazonov, since both of you reviewed and approved #12179, can you please also review this PR? |
There was a problem hiding this comment.
Our common pattern is to use consts without extra static class.
There was a problem hiding this comment.
It is preferred to use TestCases.
|
This pull request has been automatically marked as Review Needed because it has been there has not been any activity for 7 days. |
|
@aetos382 Please address my comments. |
src/Microsoft.PowerShell.Commands.Utility/commands/utility/GetDateCommand.cs
Outdated
Show resolved
Hide resolved
src/Microsoft.PowerShell.Commands.Utility/commands/utility/GetDateCommand.cs
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
It is parameter set names. So I'd add this in the consts:
| private const string DateAndFormat = "DateAndFormat"; | |
| private const string DateAndFormatParameterSet = "DateAndFormat"; |
There was a problem hiding this comment.
I think that if the parameter set name constants have the suffix ParameterSet, it would be better to make them members of the static inner class, as before.
There was a problem hiding this comment.
It is our common pattern. If you see a const with the suffix in code you understand that it is a parameter set name.
|
@iSazonov Thanks for the review! |
|
This pull request has been automatically marked as Review Needed because it has been there has not been any activity for 7 days. |
Since it was difficult to implement exclusivity between Date and UnixTimeSeconds, and Format and UFormat using parameter set, I checked Format and UFormat at runtime.
If UFormat is specified, DateTime is never returned.
To pass CodeFactor's static check, the value of the summary element now begins with an alphabetic character.
This test is not necessary because ParameterSet is used to ensure that Format and UFormat are not used together, and is not determined by an if statement.
…DateCommand.cs Co-authored-by: Ilya <[email protected]>
…DateCommand.cs Co-authored-by: Ilya <[email protected]>
Yes. I am aware from the beginning that this PR is not compatible with the 7.1.0 preview. |
|
@daxian-dbw Yes, new approach looks more good and it better to replace previous commit before RC and release. |
|
Thank you both for the clarification. |
|
@aetos382 Thanks for your contribution! |
|
@iSazonov @daxian-dbw @mklement0 @SteveL-MSFT @TravisEz13 Thank you all! |
… the FromUnixTime parameter (#6264) * Add UnixTimeSeconds parameter and Remove FromUnixTime parameter PowerShell/PowerShell#13084 * docs: change date * docs: Use full parameter name * docs: change 'added' to 'introduced' For consistency with other documents * Fix parameter set names Changed to reflect the merged PowerShell code. * Update Get-Date.md Reads better Co-authored-by: Chase Wilson <[email protected]> Co-authored-by: Sean Wheeler <[email protected]>
|
🎉 Handy links: |
|
@aetos382 Thank you so much for your work on this! I wrote #11719 and am glad to see this come to completion. It will be very useful. Also look forward to your work on Timezones in #13312 |
PR Summary
Add the
-UnixTimeSecondsparameter to theGet-Datecommand.This allows the time in seconds elapsed since 1970-01-01T00:00:00Z (aka Unix Time).
PR #12179 has already been submitted, but my proposal is more like the specification proposed in #11719.
This proposal also supports negative values (meaning time before the Unix epoch), therefore improving compatibility with the DateTimeOffset class and the Linux
dateutility.The following is an example of its use.
The parameter has
-UnixTimealias, because 'Unix Time' generally means a value expressed in seconds.Also, consider the possibility of adding a parameter in the future that takes values expressed in milliseconds.
The parameter is named
UnixTimeSeconds, notFromUnixTime.This is for consistency with the
Dateparameter (it's notFromDate).PR Context
Close #12976
Compatibility Notes
This proposal is basically compatible with PowerShell 7.0.2, but not compatible with 7.1.0-preview.4, including #12179.
It also includes a parameter set name changes. It's classified as Non-Public surface in the guideline.
PR Checklist
.h,.cpp,.cs,.ps1and.psm1files have the correct copyright headerWIP:or[ WIP ]to the beginning of the title (theWIPbot will keep its status check atPendingwhile the prefix is present) and remove the prefix when the PR is ready.