nteract / bookstore

📚 Notebook storage and publishing workflows for the masses
https://bookstore.readthedocs.io
BSD 3-Clause "New" or "Revised" License
201 stars 23 forks source link

Refactor & Test Validation and Handler Adding #134

Closed mpacer closed 5 years ago

mpacer commented 5 years ago

The biggest win in this is that this tests which endpoints & handlers are actually enabled in the handlers module. #130 does not address this, and should probably be rebased on top of this if this is merged.

mpacer commented 5 years ago

I'd like to move the code that defines which handlers are to be added into the respective modules as well (clone & publish). But I figured this is already touching enough files that it might be a lot to review.

codecov[bot] commented 5 years ago

Codecov Report

Merging #134 into master will increase coverage by 2.93%. The diff coverage is 80%.

@@            Coverage Diff             @@
##           master     #134      +/-   ##
==========================================
+ Coverage   47.32%   50.25%   +2.93%     
==========================================
  Files          11       11              
  Lines         374      386      +12     
==========================================
+ Hits          177      194      +17     
+ Misses        197      192       -5
codecov[bot] commented 5 years ago

Codecov Report

Merging #134 into master will increase coverage by 2.93%. The diff coverage is 80%.

@@            Coverage Diff             @@
##           master     #134      +/-   ##
==========================================
+ Coverage   47.32%   50.25%   +2.93%     
==========================================
  Files          11       11              
  Lines         374      386      +12     
==========================================
+ Hits          177      194      +17     
+ Misses        197      192       -5
willingc commented 5 years ago

Feel free to merge @mpacer.

mpacer commented 5 years ago

Grazie! @willingc, I made the followup changes after merging your suggestions. Is there anything more that this needs before we can merge it?

mpacer commented 5 years ago

Definitely didn't forget to change the method name everywhere causing the handlers to never be added to the web_app & all integration tests to fail… nope definitely wouldn't have done that. ;)

mpacer commented 5 years ago

Alright, merging!