elifesciences / elife-crossref-xml-generation

Crossref deposit of journal articles
MIT License
4 stars 3 forks source link

Lots of refactoring #72

Closed gnott closed 5 years ago

gnott commented 5 years ago

I was tracking this issue here https://github.com/elifesciences/elife-crossref-feed/issues/139.

I decided it was time to lint and refactor before adding new features for this library to also generate Crossref peer review deposits, which might reuse some of the existing code.

My strategy here was basically:

Please notice when reviewing this that all tests are passing, code coverage is the same (except for a reduction in lines of code drops coverage by 0.001%) and that (hopefully) in general it looks better.

There is probably more that can be done with refactoring, but this was a larger job than I intended and want to work on building a new feature before revisiting another round of linting or refactoring.

coveralls commented 5 years ago

Coverage Status

Coverage increased (+0.1%) to 99.839% when pulling 8091cdb83e8f3feda3b81f100aeb197c5072b3f6 on refactor into dc9dd335a8d6a3ad58d65c126b884bdca1abed3e on develop.

gnott commented 5 years ago

I hope this refactored code is an improvement over the original. As I described in the PR description, it is shown to function (almost) identically according to the automated test scenarios.

With so many lines of code edited, it would be very time consuming to provide a full review of the code prior to merging.

In order for further development of this library to continue, and for this branch not to go stale, I am going to merge it in now, and I can set about to adding new features.