scitran / core

RESTful API
https://scitran.github.io
MIT License
18 stars 18 forks source link

Quick fix for logging #995

Closed kofalt closed 6 years ago

kofalt commented 6 years ago

Closes #977. I keep running into this, so I hope nobody minds if this rename fixes things for now. Comments on the linked ticket suggest a better long term fix.

This was simply the smallest change, alternatives accepted. I would probably call several files in dao differently, but I didn't want to cause a lot of churn without a better reason to do so.

codecov-io commented 6 years ago

Codecov Report

:exclamation: No coverage uploaded for pull request base (master@9301a8d). Click here to learn what that means. The diff coverage is 100%.

@@            Coverage Diff            @@
##             master     #995   +/-   ##
=========================================
  Coverage          ?   90.23%           
=========================================
  Files             ?       49           
  Lines             ?     6688           
  Branches          ?        0           
=========================================
  Hits              ?     6035           
  Misses            ?      653           
  Partials          ?        0
nagem commented 6 years ago

It really doesn't make sense to call the module with the parent class container when the module with the child classes is containerstorage. I would be more willing to call it basecontainerstorage, but that is very long.

kofalt commented 6 years ago

Agreed - I would probably have gone with containerstorage and then containerstorage_implementations or cs_impls or something, but then we're renaming 2 files and changing ~30 imports instead of 1 & 3, and that seemed a bit much for code that I don't own... etc.

I'll rename it to basecontainerstorage for now then :+1:

kofalt commented 6 years ago

Updated & green

nagem commented 6 years ago

Cool, go ahead 👍