search

mattsalt / national-rail-darwin

JS module abstracting national rail's SOAP API
https://www.npmjs.com/package/national-rail-darwin
MIT License
35 stars 19 forks source link

Added support for timeOffset and timeWindow options across the board #2

Closed skwirrel closed 7 years ago

skwirrel commented 7 years ago

Hi Matt,

Really useful module, thanks. I needed support for timeOffset and timeWindow options which didn't seem to be implemented yet so I've added this in.

At the same time I was able to streamline the code a bit and remove some duplication.

Thought I might as well share it back.

Ben

mattsalt commented 7 years ago

Hi Ben,

Thanks for using the module and thanks for submitting your improvements back! Those extra options should be useful. If you don't mind me asking, what are you using the module for? It's the first one I've pushed to npm and I'm curious about how people are using it?

The simplification looks good. I spotted a typo and a console.log line that snuck in there. If you can sort those out I'll merge it.

Cheers, Matt

skwirrel commented 7 years ago

Hi Matt,

I've fixed those. Wow... Its like I'm living the open source dream! I have written one NPM module ( npmjs.com/package/frameworx ) but this kind of collaborative open source working is new to me too.

As for what I'm doing with it... I'm hoping to build an application to provide voice based rail information off the back of Google Actions i.e. "OK Google, when's the next train to London?" would give a reply like "If you can get to Brighton station by 18:45 you can catch the 18:40 departure from platform 2 which is running 5 minutes late. This is the train for Gatwick Airpoort, it should get you in to London for 19:30. Let me know if you want a later or earlier train that this."

Ultimately there would be scope for more that just "next train to X" but that's my initial goal.

I think I now have all the building blocks I need:

Hope you agree it would be a cool thing to be able to do.

Ben

On 2 June 2017 at 08:43, Matthew Salt notifications@github.com wrote:

@mattsalt commented on this pull request.

In index.js https://github.com/mattsalt/national-rail-darwin/pull/2#discussion_r119798246 :

requestXML = requestXML.replace('$$FROM$$', station)

  • if (options && options.destination) {
  • requestXML = requestXML.replace('$$FILTER$$', options.destination)
  • } else {
  • requestXML = requestXML.replace('$$FILTER$$', '')
  • } this.thenablePOST(requestXML).then(function (result) { callback(null, parser.parseArrivalsBoardResponse(result)) }).catch(function (err) {

Typo, should be callback.

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/mattsalt/national-rail-darwin/pull/2#pullrequestreview-41710089, or mute the thread https://github.com/notifications/unsubscribe-auth/ADuBXVlBX-kqVE6lU3iVj0UxRlh9zI-Cks5r_70qgaJpZM4NtsoV .

mattsalt commented 7 years ago

That does sounds cool! I would definitely use that. Good luck bolting it all together :)

Ha, I know what you're talking about. I'm always slightly surprised when this open source thing works.

Thanks for making those changes. I'm on holiday at the moment so it will take me a few days to publish this as a new version to npm. I'll let you know when its live.

Cheers, Matt

mattsalt commented 7 years ago

I've just published version 1.0.2 with your changes in it so you should be able to pick it up from the registry now. Feel free to add details of these two new options to the readme if you get time.