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

Added /road endpoint for vegagerdin #483

Open
wants to merge 5 commits into
base: master
Choose a base branch
from

Conversation

olafur164
Copy link

@olafur164 olafur164 commented Feb 23, 2019

Added a new endpoint for road conditions in Iceland.
Source: Vegagerdin

Vegagerdin xml

Also has endpoints separating each region in Iceland.
South, North, West, East and more.

Checklist

  • Write tests
  • Write documentation
  • All regions added

Copy link
Member

@MiniGod MiniGod left a comment

Choose a reason for hiding this comment

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

Thanks for the PR!

endpoints/road/all.js Outdated Show resolved Hide resolved
endpoints/road/index.js Outdated Show resolved Hide resolved
endpoints/road/all.js Outdated Show resolved Hide resolved
endpoints/road/all.js Outdated Show resolved Hide resolved
endpoints/road/all.js Outdated Show resolved Hide resolved
endpoints/road/documentation.md Outdated Show resolved Hide resolved
@MiniGod
Copy link
Member

MiniGod commented Feb 24, 2019

Do you know if we can get any info about the segments? Location or something?

@olafur164
Copy link
Author

olafur164 commented Feb 24, 2019

@MiniGod Added all the regions :)

@olafur164
Copy link
Author

@MiniGod any update on when / if this will be merged? :D

Copy link
Member

@MiniGod MiniGod left a comment

Choose a reason for hiding this comment

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

Sorry for the late reply. Got some comments about coding style mostly to make this easier to maintain in the future

@@ -0,0 +1,123 @@
/* eslint-disable */
Copy link
Member

Choose a reason for hiding this comment

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

Please remove this and follow the eslint rules

resolve(segments)
})
})
const parseFeed = function (callback, data, regionUrl) {
Copy link
Member

Choose a reason for hiding this comment

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

This function is almost the same in the index.js file. Please try to re-use functions between the files.

endpoints/road/regions.js Show resolved Hide resolved
/* eslint-disable no-plusplus */
/* eslint-disable no-prototype-builtins */
/* eslint-disable prefer-destructuring */
/* eslint-disable no-unneeded-ternary */
Copy link
Member

Choose a reason for hiding this comment

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

Please remove these and follow the eslint rules

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants