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

Handle zero hops separately if all travel times in feed are rounded #145

Merged
merged 5 commits into from
Dec 18, 2018

Conversation

landonreed
Copy link
Contributor

@landonreed landonreed commented Oct 11, 2018

This PR implements the changes suggested by @abyrd in #110 (comment).

cc @Tristramg

// If travel times are rounded and travel time is zero, determine the maximum and minimum possible speed
// by adding/removing one minute of slack.
if (bothTravelTimesRounded && travelTimeSeconds == 0)
travelTimeSeconds += 60;
Copy link
Member

Choose a reason for hiding this comment

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

Please put brackets around conditional bodies unless there's no line break.

@abyrd
Copy link
Member

abyrd commented Dec 18, 2018

@landonreed I approved the PR in principle (though there's request for a bracket change), and realized afterward that two snapshot tests are failing.

@landonreed
Copy link
Contributor Author

Thanks, @abyrd. Yes, I think I need to update the failing test/snapshot. I'll do that today so we can get this merged in.

@codecov-io
Copy link

codecov-io commented Dec 18, 2018

Codecov Report

Merging #145 into dev will decrease coverage by 0.05%.
The diff coverage is 33.33%.

Impacted file tree graph

@@             Coverage Diff              @@
##                dev     #145      +/-   ##
============================================
- Coverage     55.21%   55.15%   -0.06%     
- Complexity      734      736       +2     
============================================
  Files           143      143              
  Lines          7178     7198      +20     
  Branches        830      833       +3     
============================================
+ Hits           3963     3970       +7     
- Misses         2876     2885       +9     
- Partials        339      343       +4
Impacted Files Coverage Δ Complexity Δ
...ava/com/conveyal/gtfs/validator/TripValidator.java 100% <ø> (ø) 1 <0> (ø) ⬇️
.../java/com/conveyal/gtfs/error/SQLErrorStorage.java 71.91% <0%> (-3.39%) 12 <0> (ø)
...in/java/com/conveyal/gtfs/validator/Validator.java 64.28% <0%> (-17.54%) 4 <0> (ø)
...java/com/conveyal/gtfs/error/NewGTFSErrorType.java 100% <100%> (ø) 2 <0> (ø) ⬇️
...om/conveyal/gtfs/validator/SpeedTripValidator.java 45.76% <46.15%> (+1.08%) 7 <3> (+2) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update ce68dc6...0e15a54. Read the comment docs.

@landonreed landonreed merged commit cab706f into dev Dec 18, 2018
@landonreed landonreed deleted the travel-time-zero-fix branch December 18, 2018 20:53
@landonreed
Copy link
Contributor Author

Note: future work on rounded travel times should reference #157.

@evansiroky evansiroky mentioned this pull request Jan 3, 2019
@landonreed
Copy link
Contributor Author

🎉 This PR is included in version 4.1.2 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀

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

Successfully merging this pull request may close these issues.

3 participants