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

editoast: move rolling stock and rs with liveries to schemas to schemas #7128

Merged
merged 5 commits into from
Apr 10, 2024

Conversation

Wadjetz
Copy link
Member

@Wadjetz Wadjetz commented Apr 8, 2024

part of #7048

@Wadjetz Wadjetz added area:editoast Work on Editoast Service kind:refacto-task Task related to Refactorization Epic labels Apr 8, 2024
@Wadjetz Wadjetz self-assigned this Apr 8, 2024
@Wadjetz Wadjetz requested a review from woshilapin April 8, 2024 12:27
Copy link

codecov bot commented Apr 8, 2024

Codecov Report

Attention: Patch coverage is 94.11765% with 5 lines in your changes are missing coverage. Please review.

Project coverage is 28.34%. Comparing base (d2fdf2d) to head (31c0071).
Report is 3 commits behind head on dev.

Files Patch % Lines
front/src/modules/rollingStock/types.ts 0.00% 5 Missing ⚠️
Additional details and impacted files
@@             Coverage Diff              @@
##                dev    #7128      +/-   ##
============================================
- Coverage     28.34%   28.34%   -0.01%     
  Complexity     2247     2247              
============================================
  Files          1097     1096       -1     
  Lines        137950   137931      -19     
  Branches       2761     2754       -7     
============================================
- Hits          39103    39094       -9     
+ Misses        97247    97239       -8     
+ Partials       1600     1598       -2     
Flag Coverage Δ
core 79.12% <ø> (+0.14%) ⬆️
editoast 72.83% <100.00%> (-0.07%) ⬇️
front 8.99% <85.71%> (+0.01%) ⬆️
gateway 2.45% <ø> (ø)
railjson_generator 87.15% <ø> (ø)
tests 83.98% <ø> (ø)

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 changed the title editoast: move rolling stock to schemas editoast: move rolling stock and rs with liveries to schemas to schemas Apr 8, 2024
@Wadjetz Wadjetz force-pushed the ebe/editoast-move-rolling-stock-to-schemas branch from f67787d to e213b98 Compare April 8, 2024 12:52
@Wadjetz Wadjetz marked this pull request as ready for review April 8, 2024 13:07
@Wadjetz Wadjetz requested a review from a team as a code owner April 8, 2024 13:07
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.

Thanks for this PR. There are a few things to change, including some changes that went while I was off:

  1. As stated in the tracking issue Isolate editoast::schema into an editoast_schemas crate #7048 (comment), there should be only one definition of RollingStock in editoast_schemas.
  2. RollingStockWithLiveries is a view response, not a schema. It should be moved into views/.
  3. Please reference the tracking issues in the description of the PR, that helps the review ;)

editoast/editoast_schemas/src/rolling_stock/mod.rs Outdated Show resolved Hide resolved
Copy link
Contributor

@woshilapin woshilapin left a comment

Choose a reason for hiding this comment

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

Looks good. Thanks.

@Wadjetz Wadjetz force-pushed the ebe/editoast-move-rolling-stock-to-schemas branch 4 times, most recently from 038f823 to 31ef244 Compare April 8, 2024 15:44
@Wadjetz Wadjetz requested a review from a team as a code owner April 8, 2024 15:44
@Wadjetz Wadjetz requested review from clarani and leovalais April 8, 2024 15:45
@Wadjetz Wadjetz force-pushed the ebe/editoast-move-rolling-stock-to-schemas branch from 31ef244 to 31c0071 Compare April 8, 2024 15:53
editoast/src/views/rolling_stocks/mod.rs Outdated Show resolved Hide resolved
editoast/editoast_schemas/src/rolling_stock/mod.rs Outdated Show resolved Hide resolved
editoast/editoast_schemas/src/rolling_stock/mod.rs Outdated Show resolved Hide resolved
@Wadjetz Wadjetz requested review from a team as code owners April 9, 2024 11:20
@Wadjetz Wadjetz requested a review from leovalais April 9, 2024 11:20
@Wadjetz Wadjetz force-pushed the ebe/editoast-move-rolling-stock-to-schemas branch from c88bd70 to da19bc7 Compare April 9, 2024 12:45
@Wadjetz Wadjetz force-pushed the ebe/editoast-move-rolling-stock-to-schemas branch 2 times, most recently from 167629d to b62feaf Compare April 9, 2024 16:34
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!

editoast/src/views/rolling_stocks/mod.rs Outdated Show resolved Hide resolved
@Wadjetz Wadjetz force-pushed the ebe/editoast-move-rolling-stock-to-schemas branch from b62feaf to 7466606 Compare April 9, 2024 17:13
Copy link
Contributor

@clarani clarani left a comment

Choose a reason for hiding this comment

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

LGTM ✅

@Wadjetz Wadjetz added this pull request to the merge queue Apr 10, 2024
Merged via the queue into dev with commit 91c21db Apr 10, 2024
17 checks passed
@Wadjetz Wadjetz deleted the ebe/editoast-move-rolling-stock-to-schemas branch April 10, 2024 08:01
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 kind:refacto-task Task related to Refactorization Epic
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants