Skip to content

#4750 Fix Get-Date -UFormat %G and %g behavior #14555

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.

Sign up for GitHub

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

Jump to bottom
Merged
anmenaga merged 5 commits into from
Mar 2, 2021
Merged

#4750 Fix Get-Date -UFormat %G and %g behavior #14555

anmenaga merged 5 commits into from
Mar 2, 2021

Conversation

@brianary
Copy link
Contributor

@brianary brianary commented Jan 7, 2021 •
edited by iSazonov
Loading

PR Summary

Corrects the behavior of %G and %g (variants of the same value) in Get-Date -UFormat to match ISO 8601.

PR Context

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

PR Checklist

@ghost ghost assigned anmenaga Jan 7, 2021
@iSazonov
Copy link
Collaborator

iSazonov commented Jan 7, 2021

@brianary Please look test fails.

@iSazonov iSazonov self-requested a review January 7, 2021 14:40
@iSazonov iSazonov added the CL-General Indicates that a PR should be marked as a general cmdlet change in the Change Log label Jan 7, 2021
@brianary
Copy link
Contributor Author

brianary commented Jan 7, 2021

Tricky doing this blind.

@iSazonov
Copy link
Collaborator

iSazonov commented Jan 8, 2021 •
edited
Loading

Tricky doing this blind.

Every CI has Details button. Then you need to press "View more details on Azure Pipelines" on bottom of right column. There you can see whole build and test process. (And download artifacts too.)

@brianary
Copy link
Contributor Author

brianary commented Jan 8, 2021

Sure, I've figured out how to view the test details. I just meant I haven't been able to get a local build environment working yet, so I'm over-relying on the build server and tests still.

I'll get those names changed.

@iSazonov
Copy link
Collaborator

iSazonov commented Jan 8, 2021

I just meant I haven't been able to get a local build environment working yet

You can open new issue and share what is problems you have so that we can help you. (Of cause if you plan to continue contributing.)

@iSazonov iSazonov 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 Documentation Needed in this repo Documentation is needed in this repo labels Jan 8, 2021
@ghost ghost added the Review - Needed The PR is being reviewed label Jan 16, 2021
@ghost
Copy link

ghost commented Jan 16, 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

This was referenced Jan 24, 2021
Merged
Closed
@anmenaga anmenaga added the WG-Engine core PowerShell engine, interpreter, and runtime label Jan 28, 2021
@anmenaga
Copy link

anmenaga commented Jan 28, 2021

Would be good for WG-Engine (feels like this is the best match) to review this breaking change in Get-Date -UFormat.

@ghost ghost removed the Review - Needed The PR is being reviewed label Jan 28, 2021
@iSazonov
Copy link
Collaborator

iSazonov commented Jan 28, 2021

@anmenaga You could directly request a review from Engine person.

@ghost ghost added the Review - Needed The PR is being reviewed label Feb 4, 2021
@ghost
Copy link

ghost commented Feb 4, 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

@iSazonov
Copy link
Collaborator

iSazonov commented Feb 4, 2021

@brianary Please resolve merge conflicts.

@ghost ghost removed the Review - Needed The PR is being reviewed label Feb 4, 2021
@iSazonov iSazonov requested a review from rjmholt February 4, 2021 17:35
@iSazonov iSazonov removed the Documentation Needed in this repo Documentation is needed in this repo label Feb 4, 2021
@iSazonov
Copy link
Collaborator

iSazonov commented Feb 4, 2021

@brianary Please fix typo in your test file.

@brianary
Copy link
Contributor Author

brianary commented Feb 6, 2021

Just trying to keep up with master

@brianary
Copy link
Contributor Author

brianary commented Feb 16, 2021

Is there anything else I need to do here? I'm a little concerned that PR 7183 has been merged but this one hasn't since the two will be used together in date formatting.

@iSazonov
Copy link
Collaborator

iSazonov commented Feb 16, 2021

Is there anything else I need to do here? I'm a little concerned that PR 7183 has been merged but this one hasn't since the two will be used together in date formatting.

Waiting MSFT team approval...

@ghost ghost removed the Review - Needed The PR is being reviewed label Feb 16, 2021
rjmholt
rjmholt reviewed Feb 16, 2021
View reviewed changes
@brianary
Copy link
Contributor Author

brianary commented Feb 18, 2021

@rjmholt I've implemented your suggestions and rebased from upstream master to try and simplify merging.

@brianary
Copy link
Contributor Author

brianary commented Feb 18, 2021

Ouch, not a lot of detail on those failing tests.

@iSazonov
Copy link
Collaborator

iSazonov commented Feb 18, 2021

@brianary I have checked new tests on WSL with Linux date utility and I see we need to correct our tests.

@iSazonov iSazonov self-requested a review February 18, 2021 06:29
@brianary
Copy link
Contributor Author

brianary commented Feb 18, 2021

@iSazonov I see one of the new years was wrong. Fixing…

@rjmholt rjmholt added WG-Cmdlets-Utility cmdlets in the Microsoft.PowerShell.Utility module WG-Engine core PowerShell engine, interpreter, and runtime and removed WG-Engine core PowerShell engine, interpreter, and runtime labels Feb 18, 2021
@sdwheeler sdwheeler mentioned this pull request Feb 23, 2021
Merged
23 tasks
@ghost ghost added the Review - Needed The PR is being reviewed label Feb 26, 2021
@ghost
Copy link

ghost commented Feb 26, 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

@anmenaga anmenaga merged commit c5955a5 into PowerShell:master Mar 2, 2021
@ghost ghost removed the Review - Needed The PR is being reviewed label Mar 2, 2021
@iSazonov iSazonov added this to the 7.2.0-preview.4 milestone Mar 2, 2021
@iSazonov
Copy link
Collaborator

iSazonov commented Mar 2, 2021

@brianary Thanks for your contribution!

@brianary
Copy link
Contributor Author

brianary commented Mar 3, 2021

Happy to help!

@iSazonov iSazonov mentioned this pull request Mar 3, 2021
Closed
14 tasks
@ghost
Copy link

ghost commented Mar 16, 2021

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

Handy links:

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

Reviewers

@iSazonov iSazonov iSazonov approved these changes

+1 more reviewer

@rjmholt rjmholt rjmholt approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

@anmenaga anmenaga

Labels

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 WG-Cmdlets-Utility cmdlets in the Microsoft.PowerShell.Utility module WG-Engine core PowerShell engine, interpreter, and runtime

Projects

None yet

Milestone

7.2.0-preview.4

Development

Successfully merging this pull request may close these issues.

4 participants

@brianary @iSazonov @anmenaga @rjmholt