LibraryOfCongress / bagit-python

Work with BagIt packages from Python.
http://libraryofcongress.github.io/bagit-python
216 stars 85 forks source link

Allow bagging to destination #138

Closed nkrabben closed 1 year ago

nkrabben commented 5 years ago

Based on @runderwood #92, but reimplemented within the current codebase to address #32 and #136

Major difference, this places every source within its own bag rather than combining bag, e.g. bagit.py --destination foo/ --directory bar/ baz/ will create

foo
├──bar
│  ├── bag-info.txt
│  ├── ...
└──baz
   ├── bag-info.txt
   ├── ...

which mirrors current behavior that bagit.py --directory bar/ baz/ creates

./
├──bar
│  ├── bag-info.txt
│  ├── ...
└──baz
   ├── bag-info.txt
   ├── ...
coveralls commented 5 years ago

Coverage Status

Coverage decreased (-1.3%) to 82.019% when pulling 61769cda626c8493b6a733885a407915f4ada63f on nkrabben:master into 4b76c143e61d815043f1e8bdfbb159ce98f7d978 on LibraryOfCongress:master.

nkrabben commented 4 years ago

I've incorporated the suggested changes.

After using this branch for a few months, I changed the copy mechanism to use a single copytree call. To do this, I had to create the temporary directory name from scratch since copytree will not copy to pre-existing directories. I couldn't think of a way to cover L245-248 with a test.

nkrabben commented 4 years ago

@acdha is it okay if I add tests to cover the CLI? It would have caught the bug I fixed in 61769cd, but I hadn't seen an approach to testing in the package yet.

acdha commented 4 years ago

@acdha is it okay if I add tests to cover the CLI? It would have caught the bug I fixed in 61769cd, but I hadn't seen an approach to testing in the package yet.

Definitely – that's a big gap in the test coverage currently