Remove -Force check in ReconcilePreexistingPropertyNames method in Export-Csv.#6796
Remove -Force check in ReconcilePreexistingPropertyNames method in Export-Csv.#6796sethvs wants to merge 1 commit intoPowerShell:masterfrom
Conversation
|
Can you please restart tests? |
|
I restarted CI Travis MacOs tests. |
|
@mklement0 Could you please review the change too? |
|
My vote is for not making this change, which is certainly a breaking one: While the docs don't really say what Note that the error message you currently get with Please make it clearer up front when your changes will be breaking, and ideally create an issue before starting a PR, ideally by identifying the "bucket" that the change falls into. Also, sometimes there's an alternative to a breaking change: in #6797 you tried to simply rename a parameter, but the better course of action is to add an alias. |
|
I've opened a docs issue to request documentation of the current behavior: MicrosoftDocs/PowerShell-Docs#2392 |
|
@mklement0 Agree about -Force. |
|
Glad to hear it. The reason I was suggesting creating an issue before submitting a PR is that the issue allows a broader discussion as to whether and how to implement a change / feature first. I know that this PR was simple, but generally you run the risk of wasted effort without having consensus on what should be done first. |
PR Summary
Now, when you use
Export-Csvwith -Append parameter and new properties don't exist in existing file:Should we remove -Force check in ReconcilePreexistingPropertyNames method, so that error is thrown in both cases - when -Force:$true and -Force:$false.
Or is it better to make cmdlet overwrite existing file with new data if -Force:$true?
PR Checklist
.h,.cpp,.cs,.ps1and.psm1files have the correct copyright headerWIP:to the beginning of the title and remove the prefix when the PR is ready.[feature]if the change is significant or affects feature tests