-
Notifications
You must be signed in to change notification settings - Fork 101
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
fix: add exception for decreasing_or_equal_stop_time_distance #1895
fix: add exception for decreasing_or_equal_stop_time_distance #1895
Conversation
📝 Acceptance Test Report📋 Summary✅ The rule acceptance has passed for commit ac027d3 📊 Notices ComparisonNew Errors (0 out of 1602 datasets, ~0%) ✅No changes were detected due to the code change. Dropped Errors (0 out of 1602 datasets, ~0%) ✅No changes were detected due to the code change. New Warnings (0 out of 1602 datasets, ~0%) ✅No changes were detected due to the code change. Dropped Warnings (0 out of 1602 datasets, ~0%) ✅No changes were detected due to the code change. 🛡️ Corruption Check0 out of 1602 sources (~0 %) are corrupted. ⏱️ Performance Assessment📈 Validation TimeAssess the performance in terms of seconds taken for the validation process.
📜 Memory Consumption
|
📝 Acceptance Test Report📋 Summary✅ The rule acceptance has passed for commit a68711d 📊 Notices ComparisonNew Errors (0 out of 1602 datasets, ~0%) ✅No changes were detected due to the code change. Dropped Errors (0 out of 1602 datasets, ~0%) ✅No changes were detected due to the code change. New Warnings (0 out of 1602 datasets, ~0%) ✅No changes were detected due to the code change. Dropped Warnings (0 out of 1602 datasets, ~0%) ✅No changes were detected due to the code change. 🛡️ Corruption Check0 out of 1602 sources (~0 %) are corrupted. ⏱️ Performance Assessment📈 Validation TimeAssess the performance in terms of seconds taken for the validation process.
📜 Memory Consumption
|
📝 Acceptance Test Report📋 Summary✅ The rule acceptance has passed for commit 5c8bab6 📊 Notices ComparisonNew Errors (0 out of 1602 datasets, ~0%) ✅No changes were detected due to the code change. Dropped Errors (0 out of 1602 datasets, ~0%) ✅No changes were detected due to the code change. New Warnings (0 out of 1602 datasets, ~0%) ✅No changes were detected due to the code change. Dropped Warnings (0 out of 1602 datasets, ~0%) ✅No changes were detected due to the code change. 🛡️ Corruption Check0 out of 1602 sources (~0 %) are corrupted. ⏱️ Performance Assessment📈 Validation TimeAssess the performance in terms of seconds taken for the validation process.
📜 Memory Consumption
|
📝 Acceptance Test Report📋 Summary✅ The rule acceptance has passed for commit 4e7bb19 📊 Notices ComparisonNew Errors (0 out of 1602 datasets, ~0%) ✅No changes were detected due to the code change. Dropped Errors (0 out of 1602 datasets, ~0%) ✅No changes were detected due to the code change. New Warnings (0 out of 1602 datasets, ~0%) ✅No changes were detected due to the code change. Dropped Warnings (0 out of 1602 datasets, ~0%) ✅No changes were detected due to the code change. 🛡️ Corruption Check0 out of 1602 sources (~0 %) are corrupted. ⏱️ Performance Assessment📈 Validation TimeAssess the performance in terms of seconds taken for the validation process.
📜 Memory Consumption
|
if (prev.hasShapeDistTraveled() | ||
GtfsStopTime prev = null; | ||
GtfsStopTime curr; | ||
for (int i = 0; i < stopTimeList.size(); ++i) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nitpick: Now that it starts at 0 and i is not needed in the loop, for (var curr : stopTimeList) {
would be enough.
Don't feel you have to change it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's needed to set the first
valid prev
value.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh, I misread the feedback. Nice catch; yes, what you are proposing looks cleaner.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry I don't see where the i is used, except for curr = stopTimeList.get(i);
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM!
📝 Acceptance Test Report📋 Summary✅ The rule acceptance has passed for commit 7f858db 📊 Notices ComparisonNew Errors (0 out of 1602 datasets, ~0%) ✅No changes were detected due to the code change. Dropped Errors (0 out of 1602 datasets, ~0%) ✅No changes were detected due to the code change. New Warnings (0 out of 1602 datasets, ~0%) ✅No changes were detected due to the code change. Dropped Warnings (0 out of 1602 datasets, ~0%) ✅No changes were detected due to the code change. 🛡️ Corruption Check0 out of 1602 sources (~0 %) are corrupted. ⏱️ Performance Assessment📈 Validation TimeAssess the performance in terms of seconds taken for the validation process.
📜 Memory Consumption
|
Summary:
The change ensures that the
DecreasingOrEqualStopTimeDistanceNotice
is not triggered in specific cases where certain identifiers are missing or present.Closes #1882
Expected behavior:
Explain and/or show screenshots for how you expect the pull request to work in your testing (in case other devices exhibit different behavior).
Please make sure these boxes are checked before submitting your pull request - thanks!
gradle test
to make sure you didn't break anything