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

block rm: change "force" option to "ignore" #3709

Closed
wants to merge 1 commit into from

Conversation

kevina
Copy link
Contributor

@kevina kevina commented Feb 20, 2017

"ignore" just seams like a more appropriate name as nothing is being
"forced"

License: MIT
Signed-off-by: Kevin Atkinson k@kevina.org

"ignore" just seams like a more appropriate name as nothing is being
"forced"

License: MIT
Signed-off-by: Kevin Atkinson <k@kevina.org>
@kevina kevina added the status/in-progress In progress label Feb 20, 2017
@kevina
Copy link
Contributor Author

kevina commented Feb 20, 2017

In the original pull request for block rm @jbenet suggested block rm -f as a way to be silent about deleting non-existent blocks (see #2962 (comment)). I take he was modeling that after the rm filestore utility. I want along with it, but never really liked that option name. This, p.r. changes the name to -i or --ignore. I think this is a better name because, unlike the unix rm command. the -f option does not force anything instead it "ignores" non-exist blocks rather than outputting an error.

@whyrusleeping thoughts?

@whyrusleeping
Copy link
Member

I think that force is still the right wording for it. cc @jbenet @Kubuxu

@whyrusleeping
Copy link
Member

'force' is the correct term here i think, we are forcing the removal of a block that we could otherwise not remove

@whyrusleeping whyrusleeping removed the status/in-progress In progress label Mar 7, 2017
@kevina
Copy link
Contributor Author

kevina commented Mar 7, 2017

[note, deleted last comment since it was inaccurate and it was better to start over]

@whyrusleeping

'force' is the correct term here i think, we are forcing the removal of a block that we could otherwise not remove

That is incorrect we are not forcing anything. All --force does is silence an error message when removing non-existent blocks. The end-result with and without the --force option is the same except for the error message and the exit code.

Reopening since i think there is confusion on what the --force option actually does. If we are clear on what it does and you still like --force better then we can agree to disagree and you can close this issue.

@kevina kevina reopened this Mar 7, 2017
@kevina kevina added the status/in-progress In progress label Mar 7, 2017
@whyrusleeping
Copy link
Member

@kevina this is identical to unix rm in this case:

[whyrusleeping@rumil ~]$ rm foobar
rm: cannot remove 'foobar': No such file or directory
[whyrusleeping@rumil ~]$ echo $?
1
[whyrusleeping@rumil ~]$ rm -f foobar
[whyrusleeping@rumil ~]$ echo $?
0

@kevina
Copy link
Contributor Author

kevina commented Mar 7, 2017

Correct. Force also forces the removal of write protected files, this does not.

With that understanding if you still prefer --force go ahead and close this, just seams an odd name to me, that's all.

@whyrusleeping
Copy link
Member

Thats true... Its not 'deleting a pinned block anyways'.

I'm just not sold on ignore... @Kubuxu @lgierth thoughts?

@ghost
Copy link

ghost commented Mar 7, 2017

👎 Not a good enough reason to break existing interfaces. To us it looks like a simple rename, to a program the -f flag is just gone.

But I'm wondering why this has to fail on non-existent blocks in the first place. It could be properly idempotent. The result is the same (block non-existent), and if you need to know the block's state there's ipfs block stat. Unix's rm command can fail in various ways that you can --force your way around. block rm doesn't really have any failure modes that you can --force away.

@ghost
Copy link

ghost commented Mar 7, 2017

And similarly, ipfs add doesn't fail if the stuff is already stored -- it's idempotent.

@ghost
Copy link

ghost commented Mar 7, 2017

The only forceable thing I can imagine right now is rm'ing pinned blocks, but I don't think it matters much. We should never ever touch stuff that is pinned. Suddenly pinning would have to deal with blocks that might disappear, and that'd get very messy.

@kevina
Copy link
Contributor Author

kevina commented Mar 7, 2017

@lgierth the idempotent issue was discussed in #2962 and I agreed to use -f or -force for the idempotent behavior. I didn't think it was a very good name, but I didn't want to press the issue then. This p.r./issue exists because I hate to be stuck with a bad name.

@whyrusleeping
Copy link
Member

I think even though ignore may be a marginally better name, we've already named it --force and breaking interfaces (even though nobody is probably using them yet) is generally frowned upon.

I'm gonna go ahead and close this

@whyrusleeping whyrusleeping deleted the kevina/rm-force-ignore branch March 25, 2017 04:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants