search

conveyal / browsochrones

Create isochrones and accessibility images in the browser
MIT License
6 stars 2 forks source link

Add additional code to produce spectrograms #47

Closed mattwigway closed 7 years ago

mattwigway commented 7 years ago

Also a few other features: export the grid creation function, save the minimum and maximum values of grids, and allow disabling jsolines interpolation for debug purposes.

mattwigway commented 7 years ago

@evansiroky can we merge this?

evansiroky commented 7 years ago

I guess you can merge it. I'm not too familiar with all this code and I'm always hesitant to merge stuff without seeing stuff covered by tests. If you're confident it works then go ahead and merge, but I can't accept responsibility if this breaks something.

trevorgerhardt commented 7 years ago

@mattwigway overall looks good to me! As I've mentioned before I would prefer to use const over let wherever possible. This is especially preferable in loops and more complicated code.

const makes code easier to read: within its scope, a const variable always refers to the same object. With let there is no such guarantee. As a result, it makes sense to use let and const as follows in your ES6 code:

  • use const by default
  • only use let if rebinding is needed

(taken from https://mathiasbynens.be/notes/es6-const#const-vs-let)

mattwigway commented 7 years ago

Thanks! I'm slowly moving to using const but old habits die hard. @evansiroky I completely agree about the tests, but getting a test framework in place for a project of this size will take some time.

On Wed, Oct 26, 2016 at 10:24 PM, Trevor Gerhardt notifications@github.com wrote:

@mattwigway https://github.com/mattwigway overall looks good to me! As I've mentioned before I would prefer to use const over let wherever possible. This is especially preferable in loops and more complicated code.

const makes code easier to read: within its scope, a const variable always refers to the same object. With let there is no such guarantee. As a result, it makes sense to use let and const as follows in your ES6 code:

  • use const by default
  • only use let if rebinding is needed

(taken from https://mathiasbynens.be/notes/es6-const#const-vs-let)

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/conveyal/browsochrones/pull/47#issuecomment-256529675, or mute the thread https://github.com/notifications/unsubscribe-auth/AAimruc88dp8qEgm4g_EC7jZCSiga6f9ks5q4AtGgaJpZM4KbYmt .

Matthew Wigginton Conway [WIG-in-tun CON-way] Transportation Analytics/Open Source Washington, DC

mattwigway commented 7 years ago

Merged, after fixing let/const at @trevorgerhardt 's behest.