Skip to content

Conversation

b5
Copy link
Member

@b5 b5 commented Oct 3, 2025

Description

It's a huge footgun to silently return Ok(()), even when no tags are actually removed. Returning the number of tags removed. Also adds documentation calling out this behaviour.

Breaking Changes

breaks the return value of the tags API

Notes & open questions

Should we do this with rename, and other mutating operations?

Change checklist

  • Self-review.
  • Documentation updates following the style guide, if relevant.
  • Tests if relevant.
  • All breaking changes documented.

Copy link

github-actions bot commented Oct 3, 2025

Documentation for this PR has been generated and is available at: https://n0-computer.github.io/iroh-blobs/pr/178/docs/iroh_blobs/

Last updated: 2025-10-06T12:51:40Z

@n0bot n0bot bot added this to iroh Oct 3, 2025
@github-project-automation github-project-automation bot moved this to 🏗 In progress in iroh Oct 3, 2025
src/store/gc.rs Outdated
@@ -0,0 +1,472 @@
use std::{collections::HashSet, pin::Pin, sync::Arc};
Copy link
Collaborator

Choose a reason for hiding this comment

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

This has nothing to do with the PR, right?

Copies gc from fs to store top level. Probably related to something else?

Copy link
Collaborator

@rklaehn rklaehn left a comment

Choose a reason for hiding this comment

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

Good improvement.

Seems pretty straightforward except for the new gc.rs file that is probably? unrelated to this PR.

@rklaehn rklaehn self-requested a review October 6, 2025 12:50
Copy link
Collaborator

@rklaehn rklaehn left a comment

Choose a reason for hiding this comment

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

I can see that the gc.rs file was a stray from another PR. Removed it.

@rklaehn rklaehn marked this pull request as ready for review October 6, 2025 13:53
@rklaehn
Copy link
Collaborator

rklaehn commented Oct 6, 2025

cargo deny is just git deps!

@rklaehn rklaehn merged commit b7aa852 into main Oct 6, 2025
23 of 24 checks passed
@github-project-automation github-project-automation bot moved this from 🏗 In progress to ✅ Done in iroh Oct 6, 2025
@rklaehn rklaehn deleted the b5/delete_counter branch October 6, 2025 14:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: ✅ Done

Development

Successfully merging this pull request may close these issues.

2 participants