Skip to content

Conversation

@TomFinley
Copy link
Contributor

Fixes #1519 .

@TomFinley TomFinley added the API Issues pertaining the friendly API label Nov 3, 2018
@TomFinley TomFinley self-assigned this Nov 3, 2018
using System.Runtime.CompilerServices;

[assembly: InternalsVisibleTo(assemblyName: "Microsoft.ML.TestFramework, PublicKey=002400000480000094000000060200000024000052534131000400000100010015c01ae1f50e8cc09ba9eac9147cf8fd9fce2cfe9f8dce4f7301c4132ca9fb50ce8cbf1df4dc18dd4d210e4345c744ecb3365ed327efdbc52603faa5e21daa11234c8c4a73e51f03bf192544581ebe107adee3a34928e39d04e524a9ce729d5090bfd7dad9d10c722c0def9ccc08ff0a03790e48bcd1f9b6c476063e1966a1c4")]
[assembly: InternalsVisibleTo(assemblyName: "Microsoft.ML.CodeAnalyzer.Tests, PublicKey=002400000480000094000000060200000024000052534131000400000100010015c01ae1f50e8cc09ba9eac9147cf8fd9fce2cfe9f8dce4f7301c4132ca9fb50ce8cbf1df4dc18dd4d210e4345c744ecb3365ed327efdbc52603faa5e21daa11234c8c4a73e51f03bf192544581ebe107adee3a34928e39d04e524a9ce729d5090bfd7dad9d10c722c0def9ccc08ff0a03790e48bcd1f9b6c476063e1966a1c4")]
Copy link
Contributor Author

Choose a reason for hiding this comment

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

InternalsVisibleTo [](start = 11, length = 18)

Can remove. This was an artifact of an earlier attempt to allow the test access to the attribute via direct reference, instead of a recompilation of the source file.

private const string Category = "Access";
internal const string DiagnosticId = "MSML_NoBestFriendInternal";

private const string Title = "Cross-assembly internal access requires.";
Copy link
Member

Choose a reason for hiding this comment

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

requires what?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ha ha ha. Have I ever done this before? 😄 Thanks @eerhardt

"reference, and the declaring assembly wants these accesses to be on something " +
"with the attribute " + AttributeName + ".";
private const string Description =
"The ML.NET .";
Copy link
Member

Choose a reason for hiding this comment

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

Is this like `THE Ohio State University"? 😉

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Exactly like that, yes.


// Get the symbols of the key types we are analyzing. If we can't find any of them there is
// no point in going further.
var attrType = comp.GetTypeByMetadataName(AttributeName);
Copy link
Member

Choose a reason for hiding this comment

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

Would it make sense to rename attrType to bestFriendAttributeType? Similar for assemblyAttrType => wantsToBeBestFriendsAttributeType?

It gets a little confusing below when you see attrType, and you kind of think "which attribute are we talking about here?"

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah, that's what I was going for with the attr vs. assemblyAttr to track the two, but certainly it doesn't hurt to be more explicit.

Copy link
Member

@eerhardt eerhardt left a comment

Choose a reason for hiding this comment

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

:shipit:

Copy link
Contributor

@Zruty0 Zruty0 left a comment

Choose a reason for hiding this comment

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

:shipit:

@TomFinley TomFinley merged commit f920262 into dotnet:master Nov 5, 2018
@TomFinley TomFinley deleted the tfinley/BestFriend branch November 5, 2018 18:20
@ghost ghost locked as resolved and limited conversation to collaborators Mar 27, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

API Issues pertaining the friendly API

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants