Closed cka-y closed 2 months ago
emmambd
commented
2 months ago @tzujenchanmbd @Sergiodero Since these booking rule changes are happening faster than I expected (thanks @cka-y!) How about we split up the QA responsibility.
Seem reasonable?
github-actions[bot]
commented
2 months ago ✅ The rule acceptance has passed for commit 40fa8240778de9c5395eb7ad07aa43717b337c21 Download the full acceptance test report here (report will disappear after 90 days).
No changes were detected due to the code change.
No changes were detected due to the code change.
No changes were detected due to the code change.
No changes were detected due to the code change.
0 out of 1575 sources (~0 %) are corrupted.
Assess the performance in terms of seconds taken for the validation process.
| Time Metric | Dataset ID | Reference (s) | Latest (s) | Difference (s) | |-----------------------------|-------------------|----------------|----------------|----------------| | Average | -- | 4.07 | 4.11 | ⬆️+0.04 | | Median | -- | 1.41 | 1.45 | ⬆️+0.04 | | Standard Deviation | -- | 11.63 | 11.61 | ⬇️-0.02 | | Minimum in References Reports | us-california-flex-v2-developer-test-feed-2-gtfs-1818 | 0.49 | 0.54 | ⬆️+0.05 | | Maximum in Reference Reports | gb-unknown-uk-aggregate-feed-gtfs-2014 | 298.52 | 296.12 | ⬇️-2.40 | | Minimum in Latest Reports | us-florida-citrus-county-transit-gtfs-630 | 0.55 | 0.52 | ⬇️-0.03 | | Maximum in Latest Reports | gb-unknown-uk-aggregate-feed-gtfs-2014 | 298.52 | 296.12 | ⬇️-2.40 |
tzujenchanmbd
commented
2 months ago Based on the definition of Field and Field value in GTFS - https://gtfs.org/documentation/schedule/reference/#term-definitions, suggesting the following changes:
Notice name: forbidden_real_time_booking_field_value Description: "A forbidden field value is present for a real-time booking rule. You can see more about this notice here."
Columns displayed lgtm!
Sergiodero
commented
2 months ago LGTM as well, I just have one suggestion: I'm not sure if this will align well with other error descriptions in the validator, but I think it might be useful to explicitly point out the fact that the error is originated in the booking_rules.txt file. Feel free to ignore this if you're planning on stating this in more detail in the Validator rules
emmambd
commented
2 months ago @cka-y Yes to @Sergiodero's point - maybe we could say: "A forbidden field value is present for a real-time booking rule in booking_rules.txt. You can see more about this notice here."
cka-y
commented
2 months ago ✅ Done @Sergiodero @tzujenchanmbd I updated the screenshot in the issue description to reflect the changes
emmambd
commented
2 months ago Test data works as expected :) QA side ✅
github-actions[bot]
commented
2 months ago ✅ The rule acceptance has passed for commit 5249d3c07fc5731614fc981ba82c8671e852ea5b Download the full acceptance test report here (report will disappear after 90 days).
No changes were detected due to the code change.
No changes were detected due to the code change.
No changes were detected due to the code change.
No changes were detected due to the code change.
0 out of 1575 sources (~0 %) are corrupted.
Assess the performance in terms of seconds taken for the validation process.
| Time Metric | Dataset ID | Reference (s) | Latest (s) | Difference (s) | |-----------------------------|-------------------|----------------|----------------|----------------| | Average | -- | 4.07 | 4.11 | ⬆️+0.04 | | Median | -- | 1.44 | 1.47 | ⬆️+0.03 | | Standard Deviation | -- | 11.61 | 11.54 | ⬇️-0.07 | | Minimum in References Reports | us-florida-citrus-county-transit-gtfs-630 | 0.51 | 0.51 | ⬆️+0.01 | | Maximum in Reference Reports | gb-unknown-uk-aggregate-feed-gtfs-2014 | 295.89 | 292.09 | ⬇️-3.80 | | Minimum in Latest Reports | us-florida-citrus-county-transit-gtfs-630 | 0.51 | 0.51 | ⬆️+0.01 | | Maximum in Latest Reports | gb-unknown-uk-aggregate-feed-gtfs-2014 | 295.89 | 292.09 | ⬇️-3.80 |github-actions[bot]
commented
2 months ago ✅ The rule acceptance has passed for commit b99cacd07759e2282ef434bc47b19bf68daf3a77 Download the full acceptance test report here (report will disappear after 90 days).
No changes were detected due to the code change.
No changes were detected due to the code change.
No changes were detected due to the code change.
No changes were detected due to the code change.
0 out of 1575 sources (~0 %) are corrupted.
Assess the performance in terms of seconds taken for the validation process.
| Time Metric | Dataset ID | Reference (s) | Latest (s) | Difference (s) | |-----------------------------|-------------------|----------------|----------------|----------------| | Average | -- | 4.06 | 4.12 | ⬆️+0.06 | | Median | -- | 1.43 | 1.47 | ⬆️+0.04 | | Standard Deviation | -- | 11.53 | 11.57 | ⬆️+0.04 | | Minimum in References Reports | us-massachusetts-massachusetts-area-express-max-gtfs-431 | 0.54 | 0.53 | ⬇️-0.01 | | Maximum in Reference Reports | gb-unknown-uk-aggregate-feed-gtfs-2014 | 289.59 | 288.56 | ⬇️-1.02 | | Minimum in Latest Reports | us-california-flex-v2-developer-test-feed-3-gtfs-1819 | 0.57 | 0.52 | ⬇️-0.05 | | Maximum in Latest Reports | gb-unknown-uk-aggregate-feed-gtfs-2014 | 289.59 | 288.56 | ⬇️-1.02 |
Summary:
This PR introduces a new validation rule that triggers an
ERRORseverity notice when the following conditions are met:booking_type = 0(Real-time booking)prior_notice_duration_minprior_notice_duration_maxprior_notice_last_dayprior_notice_last_timeprior_notice_start_dayprior_notice_start_timeprior_notice_service_idExpected Behavior:
A validation notice is generated if the above conditions are met, flagging the presence of forbidden fields in a real-time booking rule.
Screenshot:
Please make sure these boxes are checked before submitting your pull request - thanks!
gradle testto make sure you didn't break anything