rejeep / evm

Emacs Version Manager
223 stars 34 forks source link

Fix uninstall #98

Closed sambrightman closed 6 years ago

sambrightman commented 6 years ago
sambrightman commented 6 years ago

@rejeep are you happy for me to proceed with PRs like this (as opposed to only committing new release versions)?

sambrightman commented 6 years ago

Since you've approved, I'll merge. If you intended to request changes before merging yourself let me know.

My thinking was actually the opposite of yours! "Test for the effects you want to see rather than how they are achieved". The fact that disuse is called is an internal implementation detail - the existing method could have been modified to do the two missing steps without calling disuse, and the test would not have to know.

rejeep commented 6 years ago

This way, if you change the definition of disuse! you also have to update lots of other non related tests. But let's agree to disagree, do as you please, not that big of a deal. 🙂