Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Appearance settings

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

Merged
anmenaga merged 5 commits into
PowerShell:masterPowerShell/PowerShell:masterfrom
brianary:UFormat_GgCopy head branch name to clipboard
Mar 2, 2021
Merged

#4750 Fix Get-Date -UFormat %G and %g behavior#14555
anmenaga merged 5 commits into
PowerShell:masterPowerShell/PowerShell:masterfrom
brianary:UFormat_GgCopy head branch name to clipboard

Conversation

@brianary

@brianary brianary commented Jan 7, 2021

Copy link
Copy Markdown
Contributor

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

iSazonov commented Jan 7, 2021

Copy link
Copy Markdown
Collaborator

@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

brianary commented Jan 7, 2021

Copy link
Copy Markdown
Contributor Author

Tricky doing this blind.

Comment thread src/Microsoft.PowerShell.Commands.Utility/commands/utility/GetDateCommand.cs Outdated
@iSazonov

iSazonov commented Jan 8, 2021

Copy link
Copy Markdown
Collaborator

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

brianary commented Jan 8, 2021

Copy link
Copy Markdown
Contributor Author

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

iSazonov commented Jan 8, 2021

Copy link
Copy Markdown
Collaborator

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

ghost commented Jan 16, 2021

Copy link
Copy Markdown

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

Copy link
Copy Markdown

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
Copy Markdown
Collaborator

@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

ghost commented Feb 4, 2021

Copy link
Copy Markdown

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

iSazonov commented Feb 4, 2021

Copy link
Copy Markdown
Collaborator

@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

iSazonov commented Feb 4, 2021

Copy link
Copy Markdown
Collaborator

@brianary Please fix typo in your test file.

@brianary

brianary commented Feb 6, 2021

Copy link
Copy Markdown
Contributor Author

Just trying to keep up with master

Comment thread test/powershell/Modules/Microsoft.PowerShell.Utility/Get-Date.Tests.ps1 Outdated
@brianary

Copy link
Copy Markdown
Contributor Author

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
Copy Markdown
Collaborator

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
Comment thread src/Microsoft.PowerShell.Commands.Utility/commands/utility/GetDateCommand.cs Outdated
Comment thread src/Microsoft.PowerShell.Commands.Utility/commands/utility/GetDateCommand.cs Outdated
Comment thread src/Microsoft.PowerShell.Commands.Utility/commands/utility/GetDateCommand.cs Outdated
Comment thread src/Microsoft.PowerShell.Commands.Utility/commands/utility/GetDateCommand.cs Outdated
Comment thread test/powershell/Modules/Microsoft.PowerShell.Utility/Get-Date.Tests.ps1 Outdated
@brianary

Copy link
Copy Markdown
Contributor Author

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

Comment thread src/Microsoft.PowerShell.Commands.Utility/commands/utility/GetDateCommand.cs Outdated
@brianary

Copy link
Copy Markdown
Contributor Author

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

@iSazonov

Copy link
Copy Markdown
Collaborator

@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
Copy Markdown
Contributor Author

@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
@ghost ghost added the Review - Needed The PR is being reviewed label Feb 26, 2021
@ghost

ghost commented Feb 26, 2021

Copy link
Copy Markdown

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

iSazonov commented Mar 2, 2021

Copy link
Copy Markdown
Collaborator

@brianary Thanks for your contribution!

@brianary

brianary commented Mar 3, 2021

Copy link
Copy Markdown
Contributor Author

Happy to help!

@ghost

ghost commented Mar 16, 2021

Copy link
Copy Markdown

🎉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

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

Development

Successfully merging this pull request may close these issues.

4 participants

Morty Proxy This is a proxified and sanitized view of the page, visit original site.