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

Bugfix: Calculate actual delta in usage #5683

Merged
merged 3 commits into from
Feb 7, 2019
Merged

Bugfix: Calculate actual delta in usage #5683

merged 3 commits into from
Feb 7, 2019

Conversation

tueksta
Copy link
Contributor

@tueksta tueksta commented Feb 6, 2019

  • Have you followed the guidelines in our Contributing document?
  • Have you checked to ensure there aren't other open Pull Requests for the same change?
  • Have you added an explanation of what your changes do and why you'd like us to include them?
  • Have you written new tests for your changes? Here's an example.
  • Have you successfully run brew style with your changes locally?
  • Have you successfully run brew tests with your changes locally?

I read the guidelines three times, I do not understand it. Most of it seems to apply to formula changes only? Maybe needs better headline/section structure.

Changes:
This fixes a bug, that when you do a cleanup and the cleanup fails for a path (e.g. no permissions to remove path), the size of that path would still be added to the cleanup estimate. By calculating the delta of disk usage before and after the removal, we get a clear picture of how much disk space was saved.

@tueksta tueksta changed the title Calculate actual delta in usage Bugfix: Calculate actual delta in usage Feb 6, 2019
@reitermarkus
Copy link
Member

the cleanup fails for a path (e.g. no permissions to remove path)

Wouldn't it abort completely in that case?

@tueksta
Copy link
Contributor Author

tueksta commented Feb 6, 2019

No, that's how I discovered this bug. Kept saying 15MB were freed, and that it couldn't delete python 3.7.2 and that python has 15MB in the path. So I wondered if it means the 15MB were freed by something else, but all successive rerans it still kept saying 15MB were cleaned and that it couldn't delete python 3.7.2.

@MikeMcQuaid MikeMcQuaid closed this Feb 7, 2019
@MikeMcQuaid MikeMcQuaid reopened this Feb 7, 2019
@MikeMcQuaid MikeMcQuaid dismissed reitermarkus’s stale review February 7, 2019 10:02

Comments addressed.

@MikeMcQuaid
Copy link
Member

Closed and reopened to re-trigger CI.

@MikeMcQuaid MikeMcQuaid merged commit 29fd1d0 into Homebrew:master Feb 7, 2019
@MikeMcQuaid
Copy link
Member

Thanks so much for your first contribution! Without people like you submitting PRs we couldn't run this project. You rock, @tueksta!

@tueksta tueksta deleted the fix-cleanup-estimate branch February 7, 2019 13:57
@lock lock bot added the outdated PR was locked due to age label Mar 9, 2019
@lock lock bot locked as resolved and limited conversation to collaborators Mar 9, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
outdated PR was locked due to age
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants