Skip to content

refactor(shared-data, api): require locatingFeaturesAsParent for module defs #18645

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

Merged
merged 2 commits into from
Jun 16, 2025

Conversation

mjhuff
Copy link
Contributor

@mjhuff mjhuff commented Jun 16, 2025

Works towards EXEC-77

Overview

In #18628, we wanted to require locatingFeaturesAsParent for module definitions, but we couldn't (see comments in PR discussion), so we marked the field as optional and followed up with #18639. Now that the module definition debacle is resolved, we can make the field required as originally intended.

Test Plan and Hands on Testing

  • Does CI pass?

Changelog

  • locatingFeaturesAsParent is required for module definitions.

Risk assessment

none, we don't do anything with this field yet

@mjhuff mjhuff requested review from SyntaxColoring and a team June 16, 2025 17:30
@mjhuff mjhuff requested review from a team as code owners June 16, 2025 17:30
Copy link

codecov bot commented Jun 16, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 25.04%. Comparing base (d26a821) to head (52300e7).
Report is 3 commits behind head on edge.

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             edge   #18645      +/-   ##
==========================================
- Coverage   25.72%   25.04%   -0.68%     
==========================================
  Files        3281     3281              
  Lines      283650   283648       -2     
  Branches    28594    28602       +8     
==========================================
- Hits        72968    71039    -1929     
- Misses     210655   212583    +1928     
+ Partials       27       26       -1     
Flag Coverage Δ
app 2.28% <ø> (-0.81%) ⬇️
opentrons-ai-client 3.18% <ø> (+<0.01%) ⬆️
protocol-designer 19.10% <ø> (ø)
step-generation 4.85% <ø> (ø)

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

Files with missing lines Coverage Δ
...-data/python/opentrons_shared_data/module/types.py 100.00% <ø> (ø)

... and 40 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Copy link
Contributor

@SyntaxColoring SyntaxColoring left a comment

Choose a reason for hiding this comment

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

Sweet, LGTM except for an Optional thing.

@mjhuff mjhuff merged commit f8ee274 into edge Jun 16, 2025
53 checks passed
@mjhuff mjhuff deleted the require-locating-features-mod-defs branch June 16, 2025 18:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants