Add config file option to set cdparanoia command#571
Add config file option to set cdparanoia command#571eharris wants to merge 3 commits intowhipper-team:developfrom
Conversation
Resolves whipper-team#220 Partially addresses whipper-team#244 Provides workaround for whipper-team#234 Signed-off-by: Evan Harris <eharris@puremagic.com>
eharris
force-pushed
the
config-opt-cdparanoia
branch
from
f623604 to
c7d02d3
Compare
Signed-off-by: Evan Harris <eharris@puremagic.com>
Signed-off-by: Evan Harris <eharris@puremagic.com>
|
Sorry for getting back to you so late on this, but I'm trying to help get these pull requests merged. I think the pull request looks fine, the only thing I don't quite like is the usage of 'global' to change the value of the variable in |
| option = self._getDriveOption(vendor, model, release, 'defeats_cache') | ||
| return option == 'True' | ||
|
|
||
| def getCdparanoia(self, vendor, model, release): |
There was a problem hiding this comment.
| def getCdparanoia(self, vendor, model, release): | |
| def getCdParanoia(self, vendor, model, release): |
Same casing as other instances.
|
The global was necessary because the cdparanoia command is used in several places, so couldn't be restricted to just one module. Is there anything keeping this from getting merged? |
I think lack of time from my side, but now also a git conflict. :D One thing to keep in mind is that I the reason that `cd-paranoia is advised to be used is because the older cdparanoia has unsolved bugs/issues. Is there another implementation that you are targetting here? |
3 participants
This allows for setting the cdparanoia executable via the config file like so:
This does not allow for setting the option via the command line, as that would have required more extensive changes, since cdparanoia is used for multiple commands. But this does now allow for code to be added to make
whippersmarter and possibly able to detect and self-configure to use the best cdparanoia version, if multiple versions are available.Resolves #220
Partially addresses #244
Provides workaround for #234