-
Notifications
You must be signed in to change notification settings - Fork 40
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
Add rolling stock related schedules list endpoint #8564
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
❗ Your organization needs to install the Codecov GitHub app to enable full functionality. Additional details and impacted files@@ Coverage Diff @@
## dev #8564 +/- ##
=============================================
+ Coverage 36.85% 67.13% +30.28%
+ Complexity 2217 2213 -4
=============================================
Files 1255 663 -592
Lines 113827 47756 -66071
Branches 3187 2085 -1102
=============================================
- Hits 41951 32063 -9888
+ Misses 69984 14894 -55090
+ Partials 1892 799 -1093
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
e4ef7b5
to
b7e0196
Compare
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.
Great! A few remarks. Don't hesitate to answer if something's not clear or if you disagree with something :)
Not tested yet.
editoast/src/modelsv2/rolling_stock_model/schedules_from_rolling_stock.rs
Outdated
Show resolved
Hide resolved
editoast/src/modelsv2/rolling_stock_model/schedules_from_rolling_stock.rs
Outdated
Show resolved
Hide resolved
editoast/src/modelsv2/rolling_stock_model/schedules_from_rolling_stock.rs
Outdated
Show resolved
Hide resolved
editoast/src/modelsv2/rolling_stock_model/schedules_from_rolling_stock.rs
Outdated
Show resolved
Hide resolved
editoast/src/modelsv2/rolling_stock_model/schedules_from_rolling_stock.rs
Outdated
Show resolved
Hide resolved
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.
Apart from the expert comments from @leovalais 😗, the rest looks good. Thank you.
61404ad
to
d549513
Compare
editoast/src/modelsv2/rolling_stock_model/schedules_from_rolling_stock.rs
Outdated
Show resolved
Hide resolved
9692f7a
to
6eefddd
Compare
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.
A few last changes. Otherwise tested and it works great! 😁
editoast/src/modelsv2/rolling_stock_model/schedules_from_rolling_stock.rs
Outdated
Show resolved
Hide resolved
editoast/src/modelsv2/rolling_stock_model/schedules_from_rolling_stock.rs
Outdated
Show resolved
Hide resolved
editoast/src/modelsv2/rolling_stock_model/schedules_from_rolling_stock.rs
Outdated
Show resolved
Hide resolved
editoast/src/modelsv2/rolling_stock_model/schedules_from_rolling_stock.rs
Outdated
Show resolved
Hide resolved
Sad to see the result of our |
8c5be5d
to
30643aa
Compare
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, thank you (I unresolved a comment).
Note
You may want to squash your commits.
This endpoint returns the list of train schedules associated with a given rolling stock.
f635f19
to
5af2430
Compare
Fixes #8078