Skip to content
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.

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

Clean-up duplicated PathInternal files #57988

Merged
merged 2 commits into from
Aug 28, 2021
Merged

Conversation

jozkee
Copy link
Member

@jozkee jozkee commented Aug 24, 2021

There were files named PathInternal*.cs under the libraries/Common/src/System/IO folder, while at the same time we have PathInternal*.cs files under libraries/System.Private.CoreLib/src/System/IO, the content in both files was more or less identical.

This PR removes duplicity by deleting/moving the files in Common to the other folder.

@jozkee jozkee added this to the 7.0.0 milestone Aug 24, 2021
@jozkee jozkee self-assigned this Aug 24, 2021
@ghost
Copy link

ghost commented Aug 24, 2021

Tagging subscribers to this area: @dotnet/area-system-io
See info in area-owners.md if you want to be subscribed.

Issue Details

There were files named PathInternal*.cs under the libraries/Common/src/System/IO folder, while at the same time we have PathInternal*.cs files under libraries/System.Private.CoreLib/src/System/IO, the content in both files was more or less identical.

This PR removes duplicity by deleting/moving the files in Common to the other folder.

Author: Jozkee
Assignees: Jozkee
Labels:

area-System.IO

Milestone: 7.0.0

@@ -11,7 +11,7 @@
<Compile Include="System\IO\Compression\ZipFileExtensions.ZipArchive.Create.cs" />
<Compile Include="System\IO\Compression\ZipFileExtensions.ZipArchive.Extract.cs" />
<Compile Include="System\IO\Compression\ZipFileExtensions.ZipArchiveEntry.Extract.cs" />
<Compile Include="$(CommonPath)System\IO\PathInternal.CaseSensitivity.cs"
<Compile Include="$(CoreLibSharedDir)System\IO\PathInternal.CaseSensitivity.cs"
Copy link
Member

Choose a reason for hiding this comment

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

Would "$(CommonPath)System\IO\PathInternal.* be more appropriate place for these files?

These files do not contain public APIs. I think we only use CoreLib includes for files with public APIs, and only include them for legacy TFM configurations.

Copy link
Member

@carlossanlop carlossanlop 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 think Jan's suggestion makes sense to move the file to the other location.

@carlossanlop
Copy link
Member

We also wanted to merge the *.Win32.cs files into the *.Windows.cs files. Do you plan on doing that separately?

@jozkee
Copy link
Member Author

jozkee commented Aug 27, 2021

We also wanted to merge the *.Win32.cs files into the *.Windows.cs files. Do you plan on doing that separately?

Yes, separate; but I think that's a broader issue that not just affects System.IO space.

Copy link
Member

@carlossanlop carlossanlop left a comment

Choose a reason for hiding this comment

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

Last commit LGTM.

@jozkee jozkee merged commit cc0c121 into dotnet:main Aug 28, 2021
@jozkee jozkee deleted the pathinternal branch August 28, 2021 04:28
@ghost ghost locked as resolved and limited conversation to collaborators Sep 27, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants