-
Notifications
You must be signed in to change notification settings - Fork 4.9k
Refactor, clean up ZipFile assembly #30364
Conversation
This is a net-zero change PR to move around some files in the System.IO.Compression.ZipFile assembly in preparation for some upcoming major additions.
| private static void DoCreateFromDirectory(string sourceDirectoryName, string destinationArchiveFileName, | ||
| CompressionLevel? compressionLevel, bool includeBaseDirectory, | ||
| Encoding entryNameEncoding) | ||
| CompressionLevel? compressionLevel, bool includeBaseDirectory, Encoding entryNameEncoding) |
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.
Nit: indenting?
| /// | ||
| /// <param name="sourceArchiveFileName">The path to the archive on the file system that is to be extracted.</param> | ||
| /// <param name="destinationDirectoryName">The path to the directory on the file system. The directory specified must not exist, but the directory that it is contained in must exist.</param> | ||
| public static void ExtractToDirectory(string sourceArchiveFileName, string destinationDirectoryName) => ExtractToDirectory(sourceArchiveFileName, destinationDirectoryName, entryNameEncoding: null, overwrite: false); |
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.
Nit: These are long lines... maybe you could split them, at least moving this to the next line and indenting.
| internal static ZipArchiveEntry DoCreateEntryFromFile(this ZipArchive destination, | ||
| string sourceFileName, string entryName, CompressionLevel? compressionLevel) | ||
| { | ||
| if (destination == null) |
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.
Nit: indenting, here and the line above
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 only just skimmed it since you said there were zero actual code changes.
| Assert.Equal(fileName, Path.GetFileName(results[0])); | ||
| } | ||
|
|
||
| #if netcoreapp |
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.
nit: put that into its separate file: ZipFile.Extract.netcoreapp.cs?
| /// <returns>A wrapper for the newly created entry.</returns> | ||
| public static ZipArchiveEntry CreateEntryFromFile(this ZipArchive destination, | ||
| string sourceFileName, string entryName, CompressionLevel compressionLevel) => | ||
| DoCreateEntryFromFile(destination, sourceFileName, entryName, compressionLevel); |
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.
and also this line
|
@dotnet-bot test OSX x64 Debug Build please ("Error during cleanup: java.lang.IllegalArgumentException: Failed to prepare archiveArtifacts step") |
Refactor, clean up ZipFile assembly Commit migrated from dotnet/corefx@c6efb24
This is a net-zero change PR to move around some files in the System.IO.Compression.ZipFile assembly in preparation for some upcoming major additions.
src/ZipFileis split up into:src/ZipFileExtensionsis split up into:ZipFileConvenienceMethods,ZipFileConvenienceMethods.netcoreapp1.1,ZipFileInvalidFileTests, andZipFileReadOpenUpdateTestsare split up by their operating class and moved into:Note that this is not a comprehensive cleanup of the ZipFile classes/members themselves, but rather a clean up and reorganization of the files. My aim is not to make any code changes yet so that I can keep this PR simple.
PTAL: @stephentoub @ViktorHofer