GV

GeezerVison

The World Wide Web, restored to 800 pixels of glory.

Home|About|Gaming cabinet|Archives|How it works|Starter sites

On this site

  • Sign in
  • ActionsCI/CD and automation
  • CodespacesCloud development environments
  • All Features
  • Documentation
  • Training
  • Certifications
  • Build with Copilot
  • Customize Copilot
  • Integrations
  • Trending repos
  • GitHub Sponsors
  • Developer articles
  • Enterprises

This view

  • Original page
  • New address

GeezerVison is showing github.com — Add PSTypeName Support for Import-Csv and ConvertFrom-Csv by markekraus · Pull Request #5389 · PowerShell/PowerShell

Conversation

@markekraus

@markekraus Mark Kraus (markekraus) commented Nov 9, 2017 •
edited
Loading

Uh oh!

There was an error while loading. Please reload this page.

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.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
iSazonov reacted with thumbs up emoji
All reactions
  • 👍 1 reaction
@markekraus
Mark Kraus (markekraus) requested review from James Truher (JamesWTruher), Paul Higinbotham (PaulHigin) and Dan Travison (dantraMSFT) as code owners November 9, 2017 00:42
iSazonov
Ilya (iSazonov) approved these changes Nov 9, 2017
View reviewed changes

@iSazonov Ilya (iSazonov) left a comment

Copy link
Copy Markdown
Collaborator

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

LGTM with one comment.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
Comment thread test/powershell/Modules/Microsoft.PowerShell.Utility/Import-Csv.Tests.ps1 Outdated

@iSazonov Ilya (iSazonov) Nov 9, 2017

Copy link
Copy Markdown
Collaborator

There was a problem hiding this comment.

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

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions

@markekraus Mark Kraus (markekraus) Nov 9, 2017

Copy link
Copy Markdown
Contributor Author

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

fixed.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
iSazonov reacted with thumbs up emoji
All reactions
  • 👍 1 reaction
@iSazonov Ilya (iSazonov) self-assigned this Nov 10, 2017
@iSazonov
Ilya (iSazonov) requested a review from Steve Lee (SteveL-MSFT) November 10, 2017 18:42
SteveL-MSFT
Steve Lee (SteveL-MSFT) approved these changes Nov 11, 2017
View reviewed changes

@SteveL-MSFT Steve Lee (SteveL-MSFT) left a comment

Copy link
Copy Markdown
Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

LGTM

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
@SteveL-MSFT Steve Lee (SteveL-MSFT) added this to the 6.0.0-RC milestone Nov 11, 2017
iSazonov
Ilya (iSazonov) reviewed Nov 11, 2017
View reviewed changes
Comment thread test/powershell/Modules/Microsoft.PowerShell.Utility/Import-Csv.Tests.ps1 Outdated

@iSazonov Ilya (iSazonov) Nov 11, 2017 •
edited
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Collaborator

There was a problem hiding this comment.

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.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions

@markekraus Mark Kraus (markekraus) Nov 11, 2017

Copy link
Copy Markdown
Contributor Author

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

fixed

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
@iSazonov

Ilya (iSazonov) commented Nov 12, 2017

Copy link
Copy Markdown
Collaborator

Steve Lee (@SteveL-MSFT) CI Travis don't report status - can PG investigate the issue?
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@SteveL-MSFT

Steve Lee (SteveL-MSFT) commented Nov 12, 2017

Copy link
Copy Markdown
Member

Ilya (@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.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@iSazonov

Ilya (iSazonov) commented Nov 13, 2017 •
edited
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Collaborator

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

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@iSazonov

Ilya (iSazonov) commented Nov 13, 2017

Copy link
Copy Markdown
Collaborator

Steve Lee (@SteveL-MSFT) It seems the problem with CI Travis coming from #5249
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@SteveL-MSFT

Steve Lee (SteveL-MSFT) commented Nov 13, 2017

Copy link
Copy Markdown
Member

Mark Kraus (@markekraus) can you rebase and pick up #5249?
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
Mark Kraus (markekraus) added 3 commits November 13, 2017 12:30
@markekraus
Add PSTypeName support for Import-Csv and ConvertFrom-Csv
3dc49e4
@markekraus
Address PR Feedback
9861854
@markekraus
Address PR Feedback
6d5af16
@markekraus
Mark Kraus (markekraus) force-pushed the ImportCsvTypeData branch from 0d9f62a to 6d5af16 Compare November 13, 2017 18:31
@markekraus

Mark Kraus (markekraus) commented Nov 13, 2017

Copy link
Copy Markdown
Contributor Author

Rebased.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@markekraus

Mark Kraus (markekraus) commented Nov 13, 2017

Copy link
Copy Markdown
Contributor Author

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

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@TravisEz13

Travis Plunk (TravisEz13) commented Nov 14, 2017

Copy link
Copy Markdown
Member

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

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@TravisEz13
Travis Plunk (TravisEz13) merged commit c832e16 into PowerShell:master Nov 14, 2017
@markekraus
Mark Kraus (markekraus) deleted the ImportCsvTypeData branch January 19, 2018 18:58
Thatgfsj (Thatgfsj) pushed a commit to Thatgfsj-contribs/PowerShell that referenced this pull request Aug 6, 2026
@markekraus @TravisEz13
Add PSTypeName Support for Import-Csv and ConvertFrom-Csv (PowerShell…
a85b6c6 
…#5389)

closes PowerShell#5134

Adds PSTypeName preservation on Import-Csv and ConvertFrom-Csv
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@iSazonov Ilya (iSazonov) iSazonov approved these changes
@SteveL-MSFT Steve Lee (SteveL-MSFT) SteveL-MSFT approved these changes
@dantraMSFT Dan Travison (dantraMSFT) Awaiting requested review from dantraMSFT
@JamesWTruher James Truher (JamesWTruher) Awaiting requested review from JamesWTruher
@PaulHigin Paul Higinbotham (PaulHigin) Awaiting requested review from PaulHigin

Uh oh!

There was an error while loading. Please reload this page.

Assignees

@iSazonov Ilya (iSazonov)

Labels

None yet

Projects

None yet

Milestone

6.0.0-RC

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

Uh oh!

There was an error while loading. Please reload this page.

4 participants

@markekraus @iSazonov @SteveL-MSFT @TravisEz13
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
You are visitor 01402503 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.