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

feat(protocol-designer): add declaration for def run() to generated Python #17482

Merged
merged 1 commit into from
Feb 10, 2025

Conversation

ddcc4
Copy link
Contributor

@ddcc4 ddcc4 commented Feb 10, 2025

Overview

This adds the def run(...) declaration to the generated Python file. AUTH-1092 We'll start filling in the run() function in future PRs.

Test Plan and Hands on Testing

Updated unit tests. Looked at generated file with feature flag turned on.

Risk assessment

Low. This just affects the exported Python file hidden under a feature flag.

@ddcc4 ddcc4 requested a review from jerader February 10, 2025 19:01
@ddcc4 ddcc4 requested a review from a team as a code owner February 10, 2025 19:01
Comment on lines +4 to +5
* Our docs call it `protocol`, which is slightly misleading since the object is not
* the protocol itself, but we'll try to stay consistent with the docs.
Copy link
Collaborator

Choose a reason for hiding this comment

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

i agree we should keep consistent even though its misleading. thanks for adding the comment in to explain this!

Copy link
Collaborator

@jerader jerader left a comment

Choose a reason for hiding this comment

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

i see the def run() when exporting 😎

Copy link

codecov bot commented Feb 10, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 26.16%. Comparing base (a8c1f16) to head (ebf995b).

Additional details and impacted files

Impacted file tree graph

@@           Coverage Diff           @@
##             edge   #17482   +/-   ##
=======================================
  Coverage   26.16%   26.16%           
=======================================
  Files        3191     3191           
  Lines      230130   230143   +13     
  Branches     9807     9808    +1     
=======================================
+ Hits        60207    60220   +13     
  Misses     169898   169898           
  Partials       25       25           
Flag Coverage Δ
protocol-designer 17.42% <100.00%> (+<0.01%) ⬆️
step-generation 4.08% <7.69%> (+<0.01%) ⬆️

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

Files with missing lines Coverage Δ
...ol-designer/src/file-data/selectors/fileCreator.ts 82.89% <100.00%> (+0.07%) ⬆️
...col-designer/src/file-data/selectors/pythonFile.ts 100.00% <100.00%> (ø)
step-generation/src/utils/pythonFormat.ts 98.00% <100.00%> (+0.04%) ⬆️

@ddcc4 ddcc4 merged commit 455774e into edge Feb 10, 2025
24 checks passed
@ddcc4 ddcc4 deleted the dc-pd-pydefrun branch February 10, 2025 19:13
caila-marashaj pushed a commit that referenced this pull request Feb 11, 2025
… Python (#17482)

# Overview

This adds the `def run(...)` declaration to the generated Python file.
AUTH-1092 We'll start filling in the `run()` function in future PRs.

## Test Plan and Hands on Testing

Updated unit tests. Looked at generated file with feature flag turned
on.

## Risk assessment

Low. This just affects the exported Python file hidden under a feature
flag.
TamarZanzouri pushed a commit that referenced this pull request Feb 11, 2025
… Python (#17482)

# Overview

This adds the `def run(...)` declaration to the generated Python file.
AUTH-1092 We'll start filling in the `run()` function in future PRs.

## Test Plan and Hands on Testing

Updated unit tests. Looked at generated file with feature flag turned
on.

## Risk assessment

Low. This just affects the exported Python file hidden under a feature
flag.
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