8000 #4750 Fix Get-Date -UFormat %G and %g behavior by brianary · Pull Request #14555 · PowerShell/PowerShell · GitHub < 8000 meta name="release" content="06fa20ea1477abb0b8a8fd9ad0efced9a2f203e9">
[go: up one dir, main page]

Skip to content

Conversation

@brianary
Copy link
Contributor
@brianary brianary commented Jan 7, 2021

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

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

@anmenaga
Copy link

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

@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

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

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

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

@brianary
Copy link
Contributor Author

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

@iSazonov
Copy link
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
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
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!

@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

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

0