Closed ric-evans closed 4 years ago
Looks like there are some conflicts with master. Those need to get resolved.
There is a lot of functionality here that deals with lots of fiddly parsing details. I'd recommend writing some unit tests to help flush out any bugs.
I'm unsure how much unit tests will help, especially since you need the actual files to get useful metadata. But if we do go that route, this stops becoming a helper script and becomes part of the main project (or a separate project).
At this point most of my complaints are addressed, and I'll list it as approved.
The only comment I'd really like to see a decision on is: https://github.com/WIPACrepo/file_catalog/pull/52#discussion_r376178609
The rest looks very good, and ready to merge.
Refactored a lot but the same logic is still there. Essentially class inheritance has replaced if-blocks for processing levels. Added support for PFFilt files.