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

Remove comment in ZipFile.Create.cs #86927

Merged
merged 2 commits into from
May 31, 2023
Merged

Conversation

IDisposable
Copy link
Contributor

@IDisposable IDisposable commented May 30, 2023

In #85491 the stream-based version of the DoCreateFromDirectory override copy-pasted comments from the string-based version

// Rely on Path.GetFullPath for validation of sourceDirectoryName and destinationArchive

The last clause of that comment is not relevant in the stream-based version (the argument is Stream destination) so deleted that clause.

It's possible that the other comment is also misleading as we test the compressionLevel argument just above (unlike in the String destinationArchive version)

// Checking of compressionLevel is passed down to DeflateStream and the IDeflater implementation
// as it is a pluggable component that completely encapsulates the meaning of compressionLevel.

In dotnet#85491 the stream-based version of the `DoCreateFromDirectory` override copy-pasted comments from the string-based version
```csharp
// Rely on Path.GetFullPath for validation of sourceDirectoryName and destinationArchive
```
The last clause of that comment is not relevant in the stream-based version (the argument is `Stream destination`) so deleted that clause.

It's possible that the other comment is also misleading as we test the `compressionLevel ` argument just above (unlike in the `String destinationArchive` version
```csharp
            // Checking of compressionLevel is passed down to DeflateStream and the IDeflater implementation
            // as it is a pluggable component that completely encapsulates the meaning of compressionLevel.
```
@ghost ghost added the community-contribution Indicates that the PR has been added by a community member label May 30, 2023
@ghost
Copy link

ghost commented May 30, 2023

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

Issue Details

In #85491 the stream-based version of the DoCreateFromDirectory override copy-pasted comments from the string-based version

// Rely on Path.GetFullPath for validation of sourceDirectoryName and destinationArchive

The last clause of that comment is not relevant in the stream-based version (the argument is Stream destination) so deleted that clause.

It's possible that the other comment is also misleading as we test the compressionLevel argument just above (unlike in the String destinationArchive version)

            // Checking of compressionLevel is passed down to DeflateStream and the IDeflater implementation
            // as it is a pluggable component that completely encapsulates the meaning of compressionLevel.
Author: IDisposable
Assignees: -
Labels:

area-System.IO.Compression, community-contribution

Milestone: -

@IDisposable
Copy link
Contributor Author

@carlossanlop, care to comment on if they second comment also should be deleted?

Co-authored-by: Carlos Sánchez López <[email protected]>
Copy link
Contributor Author

@IDisposable IDisposable 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
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.

:shipit:

@carlossanlop
Copy link
Member

CI failure is unrelated: #82637

@danmoseley
Copy link
Member

Thanks @IDisposable !

@IDisposable IDisposable deleted the patch-1 branch May 31, 2023 17:11
@ghost ghost locked as resolved and limited conversation to collaborators Jun 30, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area-System.IO.Compression community-contribution Indicates that the PR has been added by a community member
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants