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.

Sign up for GitHub

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

Just use rm for helper command to remove storage #7437

Merged
merged 1 commit into from
Sep 6, 2020
+1 −1
Merged

Just use rm for helper command to remove storage #7437

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion test/system/helpers.bash
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,7 @@ function basic_teardown() {
run_podman '?' pod rm --all --force
run_podman '?' rm --all --force

/bin/rm -rf $PODMAN_TMPDIR
Copy link
Member

Choose a reason for hiding this comment

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

The intention here was to override a possible rm alias. We once had CI break because of that (on this project), and pretty much everyone has hit that problem at least once in their career. I might feel more comfortable with command rm; but it's probably not worth incurring the wrath of CI to resubmit.

I know you must get this a lot, but it's really offensive for a distro to break fifty years of UNIX /bin precedent. I'm not blaming you, I know you're trying to work with the framework you have. I just need to vent.

Copy link
Member Author

Choose a reason for hiding this comment

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

Alright I changed it to $(command -v rm) …, I think this would also work if one has set an alias for rm. 😇

Copy link
Member

Choose a reason for hiding this comment

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

Wait, why not just command rm? $(command -v rm) seems overly rococo?

Copy link
Member Author

Choose a reason for hiding this comment

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

Makes sense, fixed that too 🙈

Copy link
Member

Choose a reason for hiding this comment

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

You could also just to \rm

Copy link
Member

Choose a reason for hiding this comment

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

You could also just to \rm

command rm -rf $PODMAN_TMPDIR
}


Expand Down