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

stdcm: towed rolling stock parameter #9383

Merged
merged 1 commit into from
Oct 31, 2024

Conversation

Wadjetz
Copy link
Member

@Wadjetz Wadjetz commented Oct 18, 2024

part of https://github.com/osrd-project/osrd-confidential/issues/368

  • Add towed rolling stock param
  • Compute physical rolling stock
  • Unit Tests

@Wadjetz Wadjetz added area:editoast Work on Editoast Service module:stdcm Short-Term DCM labels Oct 18, 2024
@Wadjetz Wadjetz self-assigned this Oct 18, 2024
@codecov-commenter
Copy link

codecov-commenter commented Oct 18, 2024

⚠️ Please install the 'codecov app svg image' to ensure uploads and comments are reliably processed by Codecov.

Codecov Report

Attention: Patch coverage is 72.52747% with 150 lines in your changes missing coverage. Please review.

Project coverage is 39.79%. Comparing base (4cb7cd3) to head (6f976ff).
Report is 6 commits behind head on dev.

Files with missing lines Patch % Lines
...edRollingStock/hooks/useFilterTowedRollingStock.ts 0.00% 61 Missing and 1 partial ⚠️
...ations/stdcm/components/StdcmForm/StdcmConsist.tsx 0.00% 41 Missing ⚠️
...lications/stdcm/hooks/useStdcmTowedRollingStock.ts 0.00% 16 Missing and 1 partial ⚠️
front/src/applications/stdcm/hooks/useStdcmForm.ts 0.00% 12 Missing ⚠️
editoast/src/views/timetable/stdcm.rs 93.65% 8 Missing ⚠️
...nt/src/applications/stdcm/utils/formatStdcmConf.ts 0.00% 4 Missing ⚠️
.../components/StdcmResults/SimulationReportSheet.tsx 0.00% 2 Missing ⚠️
front/src/applications/stdcm/views/StdcmView.tsx 0.00% 2 Missing ⚠️
front/src/reducers/osrdconf/stdcmConf/index.ts 91.66% 0 Missing and 1 partial ⚠️
front/src/reducers/osrdconf/stdcmConf/selectors.ts 0.00% 1 Missing ⚠️

❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files
@@             Coverage Diff              @@
##                dev    #9383      +/-   ##
============================================
+ Coverage     39.66%   39.79%   +0.12%     
  Complexity     2270     2270              
============================================
  Files          1300     1302       +2     
  Lines         99029    99492     +463     
  Branches       3279     3282       +3     
============================================
+ Hits          39284    39589     +305     
- Misses        57816    57971     +155     
- Partials       1929     1932       +3     
Flag Coverage Δ
core 75.06% <ø> (ø)
editoast 73.53% <97.96%> (+0.21%) ⬆️
front 10.22% <7.18%> (-0.02%) ⬇️
gateway 2.19% <ø> (ø)
osrdyne 3.28% <ø> (ø)
railjson_generator 87.49% <ø> (ø)
tests 86.71% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@Wadjetz Wadjetz force-pushed the ebe/editoast/stdcm-towed-rs-consist branch 5 times, most recently from 005a354 to b2901ec Compare October 18, 2024 13:56
@Wadjetz Wadjetz changed the title editoast: stdcm towed rolling stock parameter stdcm: towed rolling stock parameter Oct 18, 2024
@Wadjetz Wadjetz force-pushed the ebe/editoast/stdcm-towed-rs-consist branch 7 times, most recently from 852257f to a37e4e7 Compare October 21, 2024 13:39
@Wadjetz Wadjetz force-pushed the ebe/editoast/stdcm-towed-rs-consist branch 4 times, most recently from 25d0dd7 to 8d29b58 Compare October 22, 2024 07:43
@Wadjetz Wadjetz marked this pull request as ready for review October 22, 2024 07:44
@Wadjetz Wadjetz requested review from a team as code owners October 22, 2024 07:44
@Wadjetz Wadjetz requested a review from axrolld October 22, 2024 07:54
@Wadjetz Wadjetz force-pushed the ebe/editoast/stdcm-towed-rs-consist branch 2 times, most recently from d507af5 to da54a6f Compare October 22, 2024 08:11
@Wadjetz Wadjetz force-pushed the ebe/editoast/stdcm-towed-rs-consist branch 4 times, most recently from 8184487 to 50b6a35 Compare October 30, 2024 13:30
@Wadjetz Wadjetz force-pushed the ebe/editoast/stdcm-towed-rs-consist branch 7 times, most recently from 248ffab to 35170f3 Compare October 30, 2024 15:55
Copy link
Contributor

@SharglutDev SharglutDev left a comment

Choose a reason for hiding this comment

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

Lgtm and tested, great job !

@Wadjetz Wadjetz force-pushed the ebe/editoast/stdcm-towed-rs-consist branch from 35170f3 to fbc6a8c Compare October 30, 2024 16:12
Signed-off-by: Egor Berezovskiy <egor@berezify.fr>
@Wadjetz Wadjetz force-pushed the ebe/editoast/stdcm-towed-rs-consist branch from fbc6a8c to 6f976ff Compare October 30, 2024 16:12
Copy link
Contributor

@leovalais leovalais left a comment

Choose a reason for hiding this comment

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

LGTM, thanks for the changes!

Copy link
Contributor

@axrolld axrolld left a comment

Choose a reason for hiding this comment

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

I only checked the physics part and the tests but LGTM !
A few suggestions but that do not request immediate changes imo (just make sure that the front end will ensure the minimum value of mass / length).

It would be great to comment all the units of the test values in another PR though.

editoast/src/core/simulation.rs Show resolved Hide resolved
editoast/src/core/simulation.rs Show resolved Hide resolved
editoast/src/models/fixtures.rs Show resolved Hide resolved
editoast/src/models/fixtures.rs Show resolved Hide resolved
@maelysLeratRosso
Copy link

maelysLeratRosso commented Oct 31, 2024

Unresolved conversations should not block the merge.
The suggested improvements are in this issue #9528 and #9529 to be done asap

@SharglutDev SharglutDev added this pull request to the merge queue Oct 31, 2024
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Oct 31, 2024
@SharglutDev SharglutDev added this pull request to the merge queue Oct 31, 2024
Merged via the queue into dev with commit 7987efe Oct 31, 2024
24 checks passed
@SharglutDev SharglutDev deleted the ebe/editoast/stdcm-towed-rs-consist branch October 31, 2024 11:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area:editoast Work on Editoast Service module:stdcm Short-Term DCM
Projects
None yet
Development

Successfully merging this pull request may close these issues.

9 participants