Skip to content

Enable DataRow/DataRowView adapters in PowerShell Core - #4258

Merged
Dongbo Wang (daxian-dbw) merged 1 commit into
PowerShell:masterfrom
daxian-dbw:adapters
Jul 17, 2017
Merged

Enable DataRow/DataRowView adapters in PowerShell Core#4258
Dongbo Wang (daxian-dbw) merged 1 commit into
PowerShell:masterfrom
daxian-dbw:adapters

Conversation

@daxian-dbw

@daxian-dbw Dongbo Wang (daxian-dbw) commented Jul 14, 2017

Copy link
Copy Markdown
Member

Fix #4232

Enable the DataRow/DataRowView adapters in PowerShell Core by moving the implementation from ExtraAdapter.cs to CoreAdapter.cs.
Add basic tests too.

/// <summary>
/// Deals with DataRow objects
/// </summary>
internal class DataRowAdapter : PropertyOnlyAdapter

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.

Why don't we follow the one class per file practice?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Core adapters have always been in the same file. Some of them were moved to ExtraAdapter.cs previously due to missing the corresponding type in CoreCLR. Now we are gradually moving them back.

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.

The question is more about the project in general.

@daxian-dbw
Dongbo Wang (daxian-dbw) merged commit 069c7a4 into PowerShell:master Jul 17, 2017
@daxian-dbw
Dongbo Wang (daxian-dbw) deleted the adapters branch July 17, 2017 18:27
@MaximoTrinidad

Copy link
Copy Markdown

Hi Dongbo Wang (@daxian-dbw)

Any idea when can I test this change? Looking forward to give it a shot.
:)

Thanks,

@daxian-dbw

Copy link
Copy Markdown
Member Author

Maximo Trinidad (@MaximoTrinidad) The release of beta.5 will on 8/3. But you can build powershell locally to verify the fix.

Thatgfsj (Thatgfsj) pushed a commit to Thatgfsj/PowerShell that referenced this pull request Aug 6, 2026
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.

4 participants