GeezerVison

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

GeezerVison is showing github.com — Cleanup assemblyinfo by iSazonov · Pull Request #8190 · PowerShell/PowerShell

Conversation

Ilya (iSazonov) commented Nov 6, 2018 •

edited

Loading

Uh oh!

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

Copy link

Copy Markdown

Collaborator

PR Summary

Remove old code from AssemblyInfo.cs

Additional considerations

We could use IgnoreAccessChecksToAttribute instead of InternalsVisibleToAttribute.

PR Checklist

PR has a meaningful title
- Use the present tense and imperative mood when describing your changes
Summarized changes
Change is not breaking
Make sure all .h, .cpp, .cs, .ps1 and .psm1 files have the correct copyright header
This PR is ready to merge and is not Work in Progress.
- If the PR is work in progress, please add the prefix WIP: to the beginning of the title and remove the prefix when the PR is ready.
User-facing changes
- Not Applicable
- OR
- User-facing Documentation needed
  - Issue filed - Issue link:
Testing - New and feature
- Not Applicable or can only be tested interactively
- OR
- Make sure you've added a new test if existing tests do not effectively test the code changed
  - Add [feature] if the change is significant or affects feature tests

Sorry, something went wrong.

Uh oh!

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

All reactions


          Remove AssemblyInfo.cs from Microsoft.Management.Infrastructure.CimCm…

ed5bc85

…dlets

Ilya (iSazonov) requested review from Steve Lee (SteveL-MSFT), Travis Plunk (TravisEz13) and Dongbo Wang (daxian-dbw)

November 6, 2018 04:51

Ilya (iSazonov) requested a review from Andrew (anmenaga) as a code owner

November 6, 2018 04:51

Ilya (iSazonov) added 2 commits

November 6, 2018 17:06


          Remove old code from Microsoft.PowerShell.ConsoleHost

e4aa55a


          Cleanup AssemblyInfo.cs in System.Management.Automation

1172f95

Ilya (iSazonov) force-pushed the cleanup-assemblyinfo branch from ff68ef6 to 1172f95 Compare

November 6, 2018 12:07

Ilya (iSazonov) commented Nov 6, 2018

Copy link

Copy Markdown

Collaborator Author

Seems Appveyor CI temporary failed.

All reactions

Sorry, something went wrong.

Uh oh!

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

Andrew (anmenaga) commented Nov 6, 2018

Copy link

Copy Markdown

Restarted AppVeyor build.

All reactions

Sorry, something went wrong.

Uh oh!

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

Andrew (anmenaga) reviewed

Nov 8, 2018

View reviewed changes

Comment thread

src/Microsoft.Management.Infrastructure.CimCmdlets/AssemblyInfo.cs

		using System.Reflection;
		using System.Runtime.CompilerServices;

		[assembly:InternalsVisibleTo("Microsoft.Windows.DSC.CoreConfProviders,PublicKey=0024000004800000940000000602000000240000525341310004000001000100b5fc90e7027f67871e773a8fde8938c81dd402ba65b9201d60593e96c492651e889cc13f1415ebb53fac1131ae0bd333c5ee6021672d9718ea31a8aebd0da0072f25d87dba6fc90ffd598ed4da35e44c398c454307e8e33b8426143daec9f596836f97c8f74750e5975c64e2189f45def46b2a2b1247adc3652bf5c308055da9")]

Andrew (anmenaga) Nov 8, 2018

Copy link

Copy Markdown

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.

how do you know that this is not used?

Sorry, something went wrong.

Uh oh!

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

All reactions

Ilya (iSazonov) Nov 8, 2018

Copy link

Copy Markdown

Collaborator 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.

I don't know. It is for MSFT review.
I believe that internal and private visibility is permissible only in the repo. All other should use only public APIs or IgnoreAccessChecksToAttribute if needed.

Sorry, something went wrong.

Uh oh!

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

All reactions

Travis Plunk (TravisEz13) Nov 9, 2018

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.

I'll email DSC and ask.

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

Travis Plunk (TravisEz13) Nov 9, 2018

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.

I verified the DSC team still needs this.

Sorry, something went wrong.

Uh oh!

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

All reactions

Travis Plunk (TravisEz13) requested changes

Nov 9, 2018

View reviewed changes

Travis Plunk (TravisEz13) 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.

Add DSC intervals visible to back

Sorry, something went wrong.

Uh oh!

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

All reactions

Ilya (iSazonov) commented Nov 10, 2018

Copy link

Copy Markdown

Collaborator Author

Travis Plunk (@TravisEz13) Will do.
Could you please share why they need the visibility?
If they use it only for tests with precompiled dlls from the repo and use Roslyn they could migrate to using IgnoreAccessChecksToAttribute. Of cause if they still use old test system we have to keep the visibility for them.

All reactions

Sorry, something went wrong.

Uh oh!

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


          Revert visibility to DSC

218f21d

Aditya Patwardhan (adityapatwardhan) commented Nov 12, 2018

Copy link

Copy Markdown

Member

Ilya (@iSazonov) Please push a commit with [Feature] tag to execute all tests.

iSazonov reacted with thumbs up emoji

All reactions

👍 1 reaction

Sorry, something went wrong.

Uh oh!

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


          [Feature]

4c040be

Ilya (iSazonov) commented Nov 13, 2018

Copy link

Copy Markdown

Collaborator Author

Travis Plunk (@TravisEz13) Please update your review.

All reactions

Sorry, something went wrong.

Uh oh!

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

Travis Plunk (TravisEz13) commented Nov 14, 2018

Copy link

Copy Markdown

Member

They do use the visibility for testing, but some of it is legacy.

iSazonov reacted with thumbs up emoji

All reactions

👍 1 reaction

Sorry, something went wrong.

Uh oh!

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

Travis Plunk (TravisEz13) reviewed

Nov 14, 2018

View reviewed changes

Comment thread

src/System.Management.Automation/AssemblyInfo.cs

		[assembly: InternalsVisibleTo(@"Microsoft.PowerShell.GPowerShell" + @",PublicKey=0024000004800000940000000602000000240000525341310004000001000100b5fc90e7027f67871e773a8fde8938c81dd402ba65b9201d60593e96c492651e889cc13f1415ebb53fac1131ae0bd333c5ee6021672d9718ea31a8aebd0da0072f25d87dba6fc90ffd598ed4da35e44c398c454307e8e33b8426143daec9f596836f97c8f74750e5975c64e2189f45def46b2a2b1247adc3652bf5c308055da9")]
		[assembly: InternalsVisibleTo(@"Microsoft.PowerShell.ISECommon" + @",PublicKey=0024000004800000940000000602000000240000525341310004000001000100b5fc90e7027f67871e773a8fde8938c81dd402ba65b9201d60593e96c492651e889cc13f1415ebb53fac1131ae0bd333c5ee6021672d9718ea31a8aebd0da0072f25d87dba6fc90ffd598ed4da35e44c398c454307e8e33b8426143daec9f596836f97c8f74750e5975c64e2189f45def46b2a2b1247adc3652bf5c308055da9")]
		[assembly: InternalsVisibleTo(@"Microsoft.PowerShell.Editor" + @",PublicKey=0024000004800000940000000602000000240000525341310004000001000100b5fc90e7027f67871e773a8fde8938c81dd402ba65b9201d60593e96c492651e889cc13f1415ebb53fac1131ae0bd333c5ee6021672d9718ea31a8aebd0da0072f25d87dba6fc90ffd598ed4da35e44c398c454307e8e33b8426143daec9f596836f97c8f74750e5975c64e2189f45def46b2a2b1247adc3652bf5c308055da9")]
		[assembly: InternalsVisibleTo(@"powershell_ise" + @",PublicKey=0024000004800000940000000602000000240000525341310004000001000100b5fc90e7027f67871e773a8fde8938c81dd402ba65b9201d60593e96c492651e889cc13f1415ebb53fac1131ae0bd333c5ee6021672d9718ea31a8aebd0da0072f25d87dba6fc90ffd598ed4da35e44c398c454307e8e33b8426143daec9f596836f97c8f74750e5975c64e2189f45def46b2a2b1247adc3652bf5c308055da9")]

Travis Plunk (TravisEz13) Nov 14, 2018

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.

Do we know why this was here?

Sorry, something went wrong.

Uh oh!

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

All reactions

Ilya (iSazonov) Nov 14, 2018

Copy link

Copy Markdown

Collaborator 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.

They all relate to ISE. I think we can safely remove this.

Sorry, something went wrong.

Uh oh!

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

All reactions

Aditya Patwardhan (adityapatwardhan) Nov 14, 2018

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.

They were added when we had a full CLR build as well.

Sorry, something went wrong.

Uh oh!

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

All reactions

Aditya Patwardhan (adityapatwardhan) approved these changes

Nov 14, 2018

View reviewed changes

Ilya (iSazonov) assigned Travis Plunk (TravisEz13)

Nov 15, 2018

Ilya (iSazonov) commented Nov 15, 2018

Copy link

Copy Markdown

Collaborator Author

Travis Plunk (@TravisEz13) Please update your review.

All reactions

Sorry, something went wrong.

Uh oh!

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

Ilya (iSazonov) added the CL-CodeCleanup Indicates that a PR should be marked as a Code Cleanup change in the Change Log label

Nov 16, 2018

Travis Plunk (TravisEz13) approved these changes

Nov 16, 2018

View reviewed changes

Travis Plunk (TravisEz13) merged commit 2c3e935 into PowerShell:master

Nov 16, 2018

Ilya (iSazonov) deleted the cleanup-assemblyinfo branch

November 16, 2018 18:25

Ilya (iSazonov) added a commit to iSazonov/PowerShell that referenced this pull request

Nov 29, 2018


          Cleanup assemblyinfo (PowerShell#8190)

8bfe76d

Remove old code from AssemblyInfo.cs

Thatgfsj (Thatgfsj) pushed a commit to Thatgfsj-contribs/PowerShell that referenced this pull request

Aug 6, 2026


          Cleanup assemblyinfo (PowerShell#8190)

3a6524c

Remove old code from AssemblyInfo.cs

Reviewers

Aditya Patwardhan (adityapatwardhan)

adityapatwardhan approved these changes

Travis Plunk (TravisEz13)

TravisEz13 approved these changes

Dongbo Wang (daxian-dbw)

Awaiting requested review from daxian-dbw

Steve Lee (SteveL-MSFT)

Awaiting requested review from SteveL-MSFT

+1 more reviewer

Andrew (anmenaga)

anmenaga left review comments

Reviewers whose approvals may not affect merge requirements

Uh oh!

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

Assignees

Travis Plunk (TravisEz13)

Labels

CL-CodeCleanup

Indicates that a PR should be marked as a Code Cleanup change in the Change Log

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Uh oh!

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

4 participants

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.

Conversation

Ilya (iSazonov) commented Nov 6, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

PR Summary

Additional considerations

PR Checklist

Uh oh!

Ilya (iSazonov) commented Nov 6, 2018

Uh oh!

Andrew (anmenaga) commented Nov 6, 2018

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Travis Plunk (TravisEz13) left a comment

Choose a reason for hiding this comment

Uh oh!

Ilya (iSazonov) commented Nov 10, 2018

Uh oh!

Aditya Patwardhan (adityapatwardhan) commented Nov 12, 2018

Uh oh!

Ilya (iSazonov) commented Nov 13, 2018

Uh oh!

Travis Plunk (TravisEz13) commented Nov 14, 2018

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Ilya (iSazonov) commented Nov 15, 2018

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Ilya (iSazonov) commented Nov 6, 2018 •

edited

Loading

Uh oh!

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