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

Fix tar checksum calculation bug #72139

Merged
merged 4 commits into from
Jul 14, 2022
Merged

Conversation

carlossanlop
Copy link
Member

Commit 1959b35: Fix tar checksum calculation bug

I noticed that after rewriting some of the code to adapt it to the async methods, I accidentally added the final checksum instead of overriding it. I fixed it, and make sure to add a bunch of checksum calculation tests for different entry types and formats. Made sure to reuse as much code as possible in the tests.

Commit 4200f2e: Rename TarEnrty test files

I noticed some test files had the original classnames with the format at the end (TarEntryGnu, TarEntryPax...). I renamed it to the official name we approved (GnuTarEnty, PaxTarEntry...).

Commit 2016fbd: Merge await using declarations with usages

Some of the tests I submitted in the PR that implemented the Async methods, were still awaiting IAsyncDisposable objects in two lines. I just merged them into a single line.

@carlossanlop carlossanlop added this to the 7.0.0 milestone Jul 13, 2022
@carlossanlop carlossanlop self-assigned this Jul 13, 2022
@ghost
Copy link

ghost commented Jul 13, 2022

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

Issue Details

Commit 1959b35: Fix tar checksum calculation bug

I noticed that after rewriting some of the code to adapt it to the async methods, I accidentally added the final checksum instead of overriding it. I fixed it, and make sure to add a bunch of checksum calculation tests for different entry types and formats. Made sure to reuse as much code as possible in the tests.

Commit 4200f2e: Rename TarEnrty test files

I noticed some test files had the original classnames with the format at the end (TarEntryGnu, TarEntryPax...). I renamed it to the official name we approved (GnuTarEnty, PaxTarEntry...).

Commit 2016fbd: Merge await using declarations with usages

Some of the tests I submitted in the PR that implemented the Async methods, were still awaiting IAsyncDisposable objects in two lines. I just merged them into a single line.

Author: carlossanlop
Assignees: carlossanlop
Labels:

area-System.IO

Milestone: 7.0.0

Copy link
Member

@eerhardt eerhardt left a comment

Choose a reason for hiding this comment

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

LGTM

@carlossanlop carlossanlop merged commit dd92598 into dotnet:main Jul 14, 2022
@carlossanlop carlossanlop deleted the TarTests branch July 14, 2022 22:43
@ghost ghost locked as resolved and limited conversation to collaborators Aug 14, 2022
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