search

koopjs / koop-socrata

Socrata provider for Koop.
Other
8 stars 7 forks source link

ensure that logfiles created by running tests have valid filenames and are ignored by git #54

Closed jgravois closed 8 years ago

jgravois commented 8 years ago

its not exactly elegant, but this change resolves #53 and ensures that log files created by a successful run of tests

  1. reside in the test folder
  2. have valid filenames (ie: test.YYYY-MM-DD and test.error.YYYY-MM-DD)
  3. are ignored by git

if its worthwhile for me to submit a PR to koop itself w/ the change @ngoldman and i discussed via IRC, (or if i misunderstood the suggestion) please let me know.

// in koop/lib/Logger.js
var logpath = config.logfile.split('/').slice(-1, 1).join('/')

// becomes
var path = require('path')
...
var logpath = path.join(config.logfile)

right now, the only benefit i can see is making the logger more windows friendly

ref: winstonjs/winston#589

ungoldman commented 8 years ago

If it makes the log files more likely to be correct I'd say a PR to koop is better

jgravois commented 8 years ago

a PR to koop is better.

I may not have understood your suggestion, but after implementing what I outlined above in koop itself, the behavior when running Socrata tests on my own machine was not improved, so I wouldn't consider it an either/or.

jgravois commented 8 years ago

@dmfenton if you have time to review/merge this too, that'd be swell.