Use logger and with statements, don't use sys.exit

LibraryOfCongress / bagit-python

Work with BagIt packages from Python.

http://libraryofcongress.github.io/bagit-python

218 stars 85 forks source link

Use logger and with statements, don't use sys.exit #43

Closed Hwesta closed 9 years ago

Hwesta commented 9 years ago

A few small changes:

Changed to use the bagit logger, instead of the root logger (logger.foo vs logging.foo)
Removed sys.exit calls, since that will exit Python and is problematic in a library
Converted file opens to use the with statement, which automatically closes files
Reraise exceptions with raise to preserve tracebacks

Hwesta commented 9 years ago

Switched it to raise a BagError instead of returning None, since that's more consistent with other behavior.

Also ManifestErrorDetail inherits from BagError now.

edsu commented 9 years ago

Thanks for the TLC :-) unfortunately I can't merge PRs anymore. But I trust someone at LC will.

dbrunton commented 9 years ago

It looks great, and the build passes. Does this change anything wrt the version of Python required, or anything else we should note in the README?

acdha commented 9 years ago

The with statement was added in Python 2.5 which is already below the minimum supported version (2.6).

Hwesta commented 9 years ago

Not that I can think of. As noted above, the with statement was my main concern and it's pre-2.6. I can't find when re-raising was added, so I assume it's early enough (and all the builds pass).

dbrunton commented 9 years ago

Awesome, thanks a lot @Hwesta and @edsu !

acdha commented 9 years ago

:+1: from me

edsu commented 9 years ago

:cake: