read log files and produce output #2
Conversation
…hod being tested * rwj_reporter_spec.rb: get rid of placeholder test, expect real output, move to spec/lib/rwj_reporter_spec to mirror code living in lib * rwj_reporter.rb: implement RwjReporter.get_show_numbers_from_log_file, make it a static method, add explanatory comment * add test fixture
…_date_range * test for date range that includes leap day * implementation could use refactoring and comments * add fixtures
…settings, update tests
…ions, modify test to use it
|
@tingulfsen @jkeck one of you want to moige this baby? |
| end | ||
|
|
||
| def self.get_log_file_dir_from_settings | ||
| def self.log_file_dir_from_settings |
There was a problem hiding this comment.
i don't know why this is the only get_ method that rubocop complained about, and why i thought of this as an "accessor" method but not the others.
|
|
||
| def channel_counts(start_date_str, end_date_str) | ||
| @start_date_str = start_date_str | ||
| @end_date_str = end_date_str |
There was a problem hiding this comment.
This use of instance variables to set state feels odd to me. It implies that file_name cannot be called until after channel_counts since that sets the state.
I don't know that I have a solid suggestion, but maybe make methods that depend on this state being set private since we don't want somebody inadvertently calling it from the outside w/o realizing the need to invoke the channel_counts method first.
Also use a constant for fixture directory make file_name private
… if none is passed as param
|
Coverage remained the same at 98.148% when pulling e1912faccfa274cefb41518a5b4d62b50110d147 on read-single-log-file into 2f10aec on master. |
…thods can be considered private reorganize spec file
ndushay
force-pushed
the
read-single-log-file
branch
from
e1912fa
to
de1895a
Compare
|
added output directory to configurable settings; refactored per Jessie's suggestions so class methods aren't depending on order of execution of instance methods, and made a lot more methods private. |
|
Looks relatively good to me for a shameless green. @ndushay added some comments to appease me. @tingulfsen ready for your expert 👀. |
This is the meat of the script. (Still TBD: rake task to run it, cap deployment)
paired with @ndushay (and I survived [ed note: @ndushay wrote that])
connects to sul-dlss/media#7