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 'CSharpVersion7' member into public 'Language' enum#3933

Merged
mirichmo merged 1 commit into
PowerShell:masterPowerShell/PowerShell:masterfrom
iSazonov:CSharpVersion7iSazonov/PowerShell:CSharpVersion7Copy head branch name to clipboard
Jun 20, 2017
Merged

Add 'CSharpVersion7' member into public 'Language' enum#3933
mirichmo merged 1 commit into
PowerShell:masterPowerShell/PowerShell:masterfrom
iSazonov:CSharpVersion7iSazonov/PowerShell:CSharpVersion7Copy head branch name to clipboard

Conversation

@iSazonov

@iSazonov iSazonov commented Jun 5, 2017

Copy link
Copy Markdown
Collaborator

Close #3853

Public enum 'LanguageVersion' from 'Microsoft.CodeAnalysis.CSharp'
already contains 'CSharp7' member.
After moving PowerShell to .Net Core 2.0 we should add 'CSharpVersion7'
member into public 'Microsoft.PowerShell.Commands.Language' enum too.

This will allow 'Add-Type -Language CSharpVersion7'

Public enum 'LanguageVersion' from 'Microsoft.CodeAnalysis.CSharp'
already contains 'CSharp7' member.
After moving PowerShell to .Net Core 2.0 we should add 'CSharpVersion7'
member into public 'Microsoft.PowerShell.Commands.Language' enum too.

This will allow 'Add-Type -Language CSharpVersion7'
It "Public 'Language' enumeration contains all members" {
[Enum]::GetNames("Microsoft.PowerShell.Commands.Language") -join "," | Should Be "CSharp,CSharpVersion7,CSharpVersion6,CSharpVersion5,CSharpVersion4,CSharpVersion3,CSharpVersion2,CSharpVersion1,VisualBasic,JScript"
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Should we add a test case for 'Add-Type -Language CSharpVersion7'?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Also, should there be two commits (one for the product change and one for the test)?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

We checked it manually in the original Issue.
I didn't touch the functionality of this cmdlet. Besides, we don't even have tests for it right now. So I believe this is a separate huge job.
It seems @daxian-dbw is working on Add-Type. It would be good to agree on the distribution of work on tests.

I can say nothing about separate commits. 😕

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.

Two commits (one for code changes and one for tests) is not a strict requirement.

@mirichmo mirichmo self-assigned this Jun 7, 2017
@iSazonov

Copy link
Copy Markdown
Collaborator Author

@mirichmo Could you please continue with the PR?

@mirichmo mirichmo merged commit f30b06f into PowerShell:master Jun 20, 2017
@iSazonov iSazonov deleted the CSharpVersion7 branch June 20, 2017 03:00
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

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