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

Add PSTypeName Support for Import-Csv and ConvertFrom-Csv#5389

Merged
TravisEz13 merged 3 commits into
PowerShell:masterPowerShell/PowerShell:masterfrom
markekraus:ImportCsvTypeDatamarkekraus/PowerShell:ImportCsvTypeDataCopy head branch name to clipboard
Nov 14, 2017
Merged

Add PSTypeName Support for Import-Csv and ConvertFrom-Csv#5389
TravisEz13 merged 3 commits into
PowerShell:masterPowerShell/PowerShell:masterfrom
markekraus:ImportCsvTypeDatamarkekraus/PowerShell:ImportCsvTypeDataCopy head branch name to clipboard

Conversation

@markekraus

@markekraus markekraus commented Nov 9, 2017

Copy link
Copy Markdown
Contributor

closes #5134

  • Adds PSTypeName preservation on Import-Csv and ConvertFrom-Csv

A note for reviewers: the logic for adding the type name was moved to after the object's properties are populated to prevent Cannot add a member with the name "<PorpertyName>" because a member with that name already exists errors during object serialization.

@iSazonov iSazonov left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

LGTM with one comment.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Please add:

$importTypes.Count | Should Be $expectedProcessTypes.Count

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

fixed.

@iSazonov iSazonov self-assigned this Nov 10, 2017
@iSazonov iSazonov requested a review from SteveL-MSFT November 10, 2017 18:42

@SteveL-MSFT SteveL-MSFT left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM

@SteveL-MSFT SteveL-MSFT added this to the 6.0.0-RC milestone Nov 11, 2017

@iSazonov iSazonov Nov 11, 2017

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Please move the line before $importTypes[0].

And please add such check in other It.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

fixed

@iSazonov

Copy link
Copy Markdown
Collaborator

@SteveL-MSFT CI Travis don't report status - can PG investigate the issue?

@SteveL-MSFT

Copy link
Copy Markdown
Member

@iSazonov seems to be an ongoing issue with Travis-CI. Looking at the actual results, it passed on both macOS and Linux, so we are good.

@iSazonov

iSazonov commented Nov 13, 2017

Copy link
Copy Markdown
Collaborator

@SteveL-MSFT It seems I haven't permission to manually merge - (protected branch hook declined). Or I do something wrong.

@iSazonov

Copy link
Copy Markdown
Collaborator

@SteveL-MSFT It seems the problem with CI Travis coming from #5249

@SteveL-MSFT

Copy link
Copy Markdown
Member

@markekraus can you rebase and pick up #5249?

@markekraus

Copy link
Copy Markdown
Contributor Author

Rebased.

@markekraus

Copy link
Copy Markdown
Contributor Author

@SteveL-MSFT *shrugs. Rebased and the Travis CI tests passed, but they still are not updating the PR to reflect that.

@TravisEz13

Copy link
Copy Markdown
Member

Merging with macOS still not finished. Talked to other maintainers and we think the risk is low.

@TravisEz13 TravisEz13 merged commit c832e16 into PowerShell:master Nov 14, 2017
@markekraus markekraus deleted the ImportCsvTypeData branch January 19, 2018 18:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Import-Csv Should apply PSTypeNames uppon import when Type Information is present in the CSV

4 participants

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