Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

CMTA: Austin, Texas metro area public transit #58

Closed
wants to merge 5 commits into from
Closed

CMTA: Austin, Texas metro area public transit #58

wants to merge 5 commits into from

Conversation

rbxd
Copy link
Contributor

@rbxd rbxd commented Jun 3, 2018

Turns out, HAFAS is used not only by the European customers! :)

I'm not 100% confident in this code, as all this is discovered by exploratory testing of the official app & other profiles in hafas-client, but it works! Feedback is welcome of course.

p/cmta/index.js Outdated
const _parseLocation = require('../../parse/location')
const _createParseJourney = require('../../parse/journey')
const _createParseDeparture = require('../../parse/departure')
const _formatStation = require('../../format/station')
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Remove these 5 unused requires above.

@derhuerst
Copy link
Member

Thanks for your contribution. This looks very good so far!

We should have made it more visible that the next branch has a new profile format which makes it a lot easier to write profiles. I will add them to next though, so you won't have to do this.

p/cmta/index.js Outdated
rail: true
}

const formatBitmask = createFormatBitmask(modes)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This line ist duplicated at line 13.

// client.departures('000002370', {duration: 1})
// client.locations('Westgate', {results: 2})
// client.location('000005534') // Downtown light rail station
// client.nearby(30.266222, -97.746058, {distance: 60})
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should be an object. Check the docs for nearby().

p/cmta/index.js Outdated
transformReqBody,
formatProducts,
parseProducts: createParseBitmask(modes.allProducts, defaultProducts),
radar: true
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The journey leg/trip ID feature is supported by the endpoint, so please add journeyLeg: true here. This will make the client.journeyLeg() method available.

@rbxd
Copy link
Contributor Author

rbxd commented Jun 4, 2018

Thanks for your feedback @derhuerst !
One more thing - just realized that there are in fact tests for particular profiles.
Hold on, I'll add some tests for CMTA as well.

@derhuerst
Copy link
Member

One more thing - just realized that there are in fact tests for particular profiles.
Hold on, I'll add some tests for CMTA as well.

If you want to, tests are very welcome.

I'm rewriting the test architecture though, so i'd add them later anyways.

@derhuerst derhuerst mentioned this pull request Jun 4, 2018
@derhuerst
Copy link
Member

Please also add an entry to readme.md.

@derhuerst
Copy link
Member

We've merged the tests PR (#57) into next.

@nickturskyi Do you want to write tests or should I do it?

@rbxd
Copy link
Contributor Author

rbxd commented Jun 19, 2018

@derhuerst I'm going to add tests if that's not something you already started working on.

@derhuerst
Copy link
Member

Didn't start yet.

@derhuerst
Copy link
Member

Started working on it. Adapted your code to the latest version of hafas-client.

Closing in favor of #79.

@derhuerst derhuerst closed this Aug 24, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants