Skip to content

Conversation

@brianary
Copy link
Contributor

@brianarybrianary commented Jan 5, 2021

PR Summary

Corrects the behavior of %u in Get-Date -UFormat to match ISO 8601.

PR Context

This is to fix one of the format issues in #4750.

PR Checklist

@ghostghost assigned rjmholtJan 5, 2021
@ghost
Copy link

ghost commented Jan 5, 2021

CLA assistant check
All CLA requirements met.

@brianary
Copy link
ContributorAuthor

I can't really access the details, but the test failures appear to be unrelated.

@iSazonoviSazonov added Breaking-Change breaking change that may affect users CL-BreakingChange Indicates that a PR should be marked as a breaking change in the Change Log CL-General Indicates that a PR should be marked as a general cmdlet change in the Change Log Documentation Needed in this repo Documentation is needed in this repo labels Jan 6, 2021
Comment on lines +100 to +105
It "using -uformat 'u' produces the correct output" -TestCases @(
@{date="1998-01-02" dayOfWeek = "5"},
@{date="1998-01-03" dayOfWeek = "6"},
@{date="2003-01-03" dayOfWeek = "5"},
@{date="2004-01-02" dayOfWeek = "5"},
Copy link
Collaborator

Choose a reason for hiding this comment

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

Please clarify how you select the test values (whole list)?

Copy link
ContributorAuthor

@brianarybrianaryJan 6, 2021

Choose a reason for hiding this comment

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

I just used the values from the previous test for %V, which is related, then added a couple recent dates that had led to discovering the bug. I'll add a comment to that effect, or else select some other dates.

@ghostghost added the Review - Needed The PR is being reviewed label Jan 14, 2021
@ghost
Copy link

This pull request has been automatically marked as Review Needed because it has been there has not been any activity for 7 days.
Maintainer, please provide feedback and/or mark it as Waiting on Author

@brianary
Copy link
ContributorAuthor

Let me know if there's anything further I can provide.

@iSazonoviSazonov added the Review - Committee The PR/Issue needs a review from the PowerShell Committee label Jan 15, 2021
@iSazonov
Copy link
Collaborator

@brianary Since it is a breaking change I ask PowerShell Committee to approve.

@ghostghost removed the Review - Needed The PR is being reviewed label Jan 15, 2021
@SteveL-MSFTSteveL-MSFT added Committee-Reviewed PS-Committee has reviewed this and made a decision and removed Review - Committee The PR/Issue needs a review from the PowerShell Committee labels Jan 20, 2021
@SteveL-MSFT
Copy link
Member

@PowerShell/powershell-committee reviewed this, we agreed that this is a bug that should be fixed and this is likely a bucket 3 compatibility issue.

@brianary
Copy link
ContributorAuthor

brianary commented Jan 24, 2021

@SteveL-MSFT should #14555 also be taken with this one, since they are so tightly related?

@iSazonov
Copy link
Collaborator

@brianary Please create new issue in PowerShell-Doc repository and add a reference to the issue in the PR description.

@ghost
Copy link

ghost commented Feb 2, 2021

This pull request has been automatically marked as Review Needed because it has been there has not been any activity for 7 days.
Maintainer, please provide feedback and/or mark it as Waiting on Author

@ghostghost added the Review - Needed The PR is being reviewed label Feb 2, 2021
@iSazonov
Copy link
Collaborator

@brianary Please rebase. I hope after that CI-Windows will pass.

@ghostghost removed the Review - Needed The PR is being reviewed label Feb 3, 2021
@iSazonoviSazonov removed the Documentation Needed in this repo Documentation is needed in this repo label Feb 4, 2021
@iSazonoviSazonov merged commit 61fea6c into PowerShell:masterFeb 4, 2021
@iSazonoviSazonov added this to the 7.2.0-preview.3 milestone Feb 4, 2021
@iSazonov
Copy link
Collaborator

@brianary Thanks for your contribution!

@ghost
Copy link

🎉v7.2.0-preview.3 has been released which incorporates this pull request.:tada:

Handy links:

Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Breaking-Changebreaking change that may affect usersCL-BreakingChangeIndicates that a PR should be marked as a breaking change in the Change LogCL-GeneralIndicates that a PR should be marked as a general cmdlet change in the Change LogCommittee-ReviewedPS-Committee has reviewed this and made a decision

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants

@brianary@iSazonov@SteveL-MSFT@rjmholt@BrianL-STCU