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

Move files to shared partition#10830

Merged
jkotas merged 1 commit intodotnet:masterfrom
jkotas:shared
Apr 9, 2017
Merged

Move files to shared partition#10830
jkotas merged 1 commit intodotnet:masterfrom
jkotas:shared

Conversation

@jkotas
Copy link
Member

@jkotas jkotas commented Apr 9, 2017

Moved a batch of files to shared CoreLib partition, applying formatting and other changes from CoreRT as appropriate.

@jkotas jkotas requested review from a user and alexperovich April 9, 2017 06:03
@jkotas jkotas force-pushed the shared branch 3 times, most recently from 9514311 to 8275f9a Compare April 9, 2017 06:35
Copy link
Member

@danmoseley danmoseley left a comment

Choose a reason for hiding this comment

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

LGTM. I diffed every file. thanks for doing this!

@@ -21,7 +21,7 @@ namespace System
// message describing what was wrong and which parameter is incorrect.
//
[Serializable]
public class ArgumentException : SystemException, ISerializable
Copy link
Member

Choose a reason for hiding this comment

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

removed because it's on the baseclass alreayd?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes - it is not necessary. It was removed in CoreRT, and it is consistent with most other CoreLib exception types.

else
{
// do any desired fixups to classname here.
return SR.Format(SR.MissingField_Name, (Signature != null ? FormatSignature(Signature) + " " : "") + ClassName + "." + MemberName);
Copy link
Member

Choose a reason for hiding this comment

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

Looks like you fixed a bug in the ordering

Copy link
Member Author

Choose a reason for hiding this comment

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

Hmm, this is implemented differently between CoreCLR and CoreRT. Not sure which side is right. I will exclude this file from this batch.

@@ -2,7 +2,6 @@
// The .NET Foundation licenses this file to you under the MIT license.
// See the LICENSE file in the project root for more information.

//
Copy link
Member

Choose a reason for hiding this comment

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

nit, you didn't delete the purpose comment in this one.

Copy link
Member Author

Choose a reason for hiding this comment

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

I have been reconciling diffs between CoreCLR and CoreRT, not much beyond that. The purpose comment was on both sides so I have kept it.

We have the purpose comment in number of files in the shared partition. I have deleted it only if it was deleted in CoreRT and it had zero information value.

@jkotas jkotas merged commit 1a9781b into dotnet:master Apr 9, 2017
dotnet-bot pushed a commit to dotnet/corert that referenced this pull request Apr 9, 2017
Signed-off-by: dotnet-bot <dotnet-bot@microsoft.com>
jkotas added a commit to dotnet/corert that referenced this pull request Apr 9, 2017
Signed-off-by: dotnet-bot <dotnet-bot@microsoft.com>
@jkotas jkotas deleted the shared branch April 10, 2017 01:52
@karelz karelz modified the milestone: 2.0.0 Aug 28, 2017
dotnet-bot pushed a commit to dotnet/corefx that referenced this pull request Jan 13, 2018
Signed-off-by: dotnet-bot-corefx-mirror <dotnet-bot@microsoft.com>
dotnet-bot pushed a commit to dotnet/corefx that referenced this pull request Jan 13, 2018
Signed-off-by: dotnet-bot-corefx-mirror <dotnet-bot@microsoft.com>
safern pushed a commit to dotnet/corefx that referenced this pull request Jan 16, 2018
Signed-off-by: dotnet-bot-corefx-mirror <dotnet-bot@microsoft.com>
safern pushed a commit to dotnet/corefx that referenced this pull request Jan 16, 2018
Signed-off-by: dotnet-bot-corefx-mirror <dotnet-bot@microsoft.com>
@glenn-slayden
Copy link

@jkotas I got lost in GitHub :-( Unfortunately, it's a common occurrence for me.

Did I put the following comment in the right place? (wait for the in-page jump # after clicking...)

1a9781b68b8ec66c223bb96f44ae1d2704ce6fdf#r33421958

It doesn't seem worth entering an issue on.

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.

5 participants