This repository has been archived by the owner. It is now read-only.

Try to remove a single .DS_Store when uninstalling #15975

Closed
wants to merge 1 commit into
from

Conversation

Projects
None yet
3 participants
@adamv
Contributor

adamv commented Nov 11, 2012

Fixes #12976.

@jacknagel

View changes

Library/Homebrew/extend/pathname.rb
+ (self/'.DS_Store').unlink
+ rmdir
+ true
+ end

This comment has been minimized.

Show comment Hide comment
@jacknagel

jacknagel Nov 12, 2012

Contributor

Do we need to return here to avoid raising the exception or returning false? Or maybe retry rather than duplicating the rmdir; true?

@jacknagel

jacknagel Nov 12, 2012

Contributor

Do we need to return here to avoid raising the exception or returning false? Or maybe retry rather than duplicating the rmdir; true?

This comment has been minimized.

Show comment Hide comment
@adamv

adamv Nov 12, 2012

Contributor

Turns out the return value is only used in a unit test

@adamv

adamv Nov 12, 2012

Contributor

Turns out the return value is only used in a unit test

This comment has been minimized.

Show comment Hide comment
@jacknagel

jacknagel Nov 12, 2012

Contributor

Still, as it is this evaluates to false:

$ mkdir /tmp/foo
$ touch /tmp/foo/.DS_Store
$ brew irb
irb(main):001:0> p = Pathname.new("/tmp/foo")
=> #<Pathname:/tmp/foo>
irb(main):002:0> p.directory?
=> true
irb(main):003:0> p.rmdir_if_possible
=> false
irb(main):004:0> p.directory?
=> false

it should return early or just replace rmdir; true with retry.

@jacknagel

jacknagel Nov 12, 2012

Contributor

Still, as it is this evaluates to false:

$ mkdir /tmp/foo
$ touch /tmp/foo/.DS_Store
$ brew irb
irb(main):001:0> p = Pathname.new("/tmp/foo")
=> #<Pathname:/tmp/foo>
irb(main):002:0> p.directory?
=> true
irb(main):003:0> p.rmdir_if_possible
=> false
irb(main):004:0> p.directory?
=> false

it should return early or just replace rmdir; true with retry.

@adamv

This comment has been minimized.

Show comment Hide comment
@adamv

adamv Nov 12, 2012

Contributor

Changed.

Contributor

adamv commented Nov 12, 2012

Changed.

@Sharpie

This comment has been minimized.

Show comment Hide comment
@Sharpie

Sharpie Nov 13, 2012

Contributor

Looks good to me.

Contributor

Sharpie commented Nov 13, 2012

Looks good to me.

@adamv adamv closed this Nov 14, 2012

adamv added a commit that referenced this pull request Nov 14, 2012

manboubird added a commit to manboubird/homebrew that referenced this pull request Nov 25, 2012

manboubird added a commit to manboubird/homebrew that referenced this pull request Nov 25, 2012

Sharpie pushed a commit to Sharpie/homebrew that referenced this pull request Dec 2, 2012

snakeyroc3 pushed a commit to snakeyroc3/homebrew that referenced this pull request Dec 17, 2012

@xu-cheng xu-cheng locked and limited conversation to collaborators Feb 16, 2016

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.