Skip to content
This repository was archived by the owner on Jan 23, 2023. It is now read-only.

Make member visibility first accross corlib #16836

Merged
merged 1 commit into from
Mar 8, 2018

Conversation

jkotas
Copy link
Member

@jkotas jkotas commented Mar 8, 2018

@jkotas jkotas force-pushed the visibility-first branch from 21a679e to 393035c Compare March 8, 2018 16:00
@jkotas jkotas force-pushed the visibility-first branch from 393035c to 4ad5877 Compare March 8, 2018 16:10
{
return InternalToObject(&value);
}

[MethodImplAttribute(MethodImplOptions.InternalCall)]
internal unsafe extern static Object InternalToObject(void* value);
internal static extern unsafe Object InternalToObject(void* value);
Copy link
Member

Choose a reason for hiding this comment

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

We don't seem to specify ordering beyond visibility first. Seems like we're settling on something.

It reminds me of https://en.wikipedia.org/wiki/Adjective#Order

Copy link
Member Author

Choose a reason for hiding this comment

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

The coding guidelines specify order for readonly. For the unspecified ones, I have unified them on the order that majority of the code in corefx has been using.

@jkotas jkotas merged commit 57ee22e into dotnet:master Mar 8, 2018
dotnet-bot pushed a commit to dotnet/corert that referenced this pull request Mar 8, 2018
dotnet-bot pushed a commit to dotnet/corefx that referenced this pull request Mar 8, 2018
jkotas pushed a commit to dotnet/corert that referenced this pull request Mar 8, 2018
stephentoub pushed a commit to dotnet/corefx that referenced this pull request Mar 8, 2018
@jkotas jkotas deleted the visibility-first branch March 12, 2018 04:29
kbaladurin pushed a commit to kbaladurin/corert that referenced this pull request Mar 15, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants