Skip to content

Remove the 'VisualBasic' support from Add-Type #7284

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
adityapatwardhan merged 1 commit into from
Jul 13, 2018
Merged

Remove the 'VisualBasic' support from Add-Type #7284

adityapatwardhan merged 1 commit into from
Jul 13, 2018

Conversation

@daxian-dbw
Copy link
Member

@daxian-dbw daxian-dbw commented Jul 13, 2018 •
edited
Loading

PR Summary

Reference: #6141 (comment)
There was a discussion about whether Add-Type should support Visual Basic, and the committee inclines to remove the Visual Basic support from Add-Type for three reasons:

  1. Compiling VB source code with Add-Type is very rare (if there is any)
  2. Extra disk footprint (5mb) brought by Microsoft.CodeAnalysis.VisualBasic.dll
  3. It's always easy to bring the VB support back in case there is a strong user demand, but it would be impossible to remove if the VB support is included in the 6.1 official release.

This PR is to remove the VB support from Add-Type.

PR Checklist

@daxian-dbw daxian-dbw added the Breaking-Change breaking change that may affect users label Jul 13, 2018
@iSazonov
Copy link
Collaborator

iSazonov commented Jul 13, 2018

There is one CodeFactor issue.

@daxian-dbw
Copy link
Member Author

daxian-dbw commented Jul 13, 2018

@iSazonov The CodeFactor reports String was used instead of string. There are 33 instances of String in this file. Changing them all will make it harder to review. I think the style changes can go to a separate issue. Actually, String is used heavily in our code base, especially when calling static methods/properties on string. It's probably better off to disable this rule.

@iSazonov
Copy link
Collaborator

iSazonov commented Jul 13, 2018 •
edited
Loading

@daxian-dbw Seems we already fixed some such issues. Also as I meantioned I compiled CodeFormatter with latest Roslyn and open PR #7263 to show results. We could add new rules to CodeFormatter to fix some code issues. I think it's better for communities and the project.

@adityapatwardhan adityapatwardhan merged commit ab07b85 into PowerShell:master Jul 13, 2018
@daxian-dbw daxian-dbw deleted the removeVB branch July 13, 2018 20:52
@daxian-dbw daxian-dbw mentioned this pull request Jul 13, 2018
Merged
11 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@SteveL-MSFT SteveL-MSFT SteveL-MSFT approved these changes

@adityapatwardhan adityapatwardhan adityapatwardhan approved these changes

@iSazonov iSazonov iSazonov approved these changes

Assignees

@adityapatwardhan adityapatwardhan

Labels

Breaking-Change breaking change that may affect users

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@daxian-dbw @iSazonov @SteveL-MSFT @adityapatwardhan