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

Adding custom entrypoint option for charting #911

Open
wants to merge 21 commits into
base: main
Choose a base branch
from

Conversation

findolor
Copy link
Collaborator

Closes #675

Caution

Do not merge before #887

Motivation

See issue: #675

Solution

  • Added entrypoint field to Scenario
  • Use the new entrypoint in the fuzzer, fallback to calculate-io and handle-io if does not exist

Checks

By submitting this for review, I'm confirming I've done the following:

  • made this PR as small as possible
  • unit-tested any new functionality
  • linked any relevant issues or PRs

@findolor findolor requested a review from hardyjosh September 30, 2024 16:10
@findolor findolor self-assigned this Sep 30, 2024
Copy link
Contributor

@thedavidmeister thedavidmeister left a comment

Choose a reason for hiding this comment

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

screenshots and tests?

@findolor
Copy link
Collaborator Author

findolor commented Oct 14, 2024

@thedavidmeister

the tests are here:

async fn test_custom_entrypoint() {

Here is an image of a simple order that contains a scenario with a custom entrypoint and the stack values for that:

Screenshot 2024-10-14 at 20 15 33

Copy link
Contributor

@thedavidmeister thedavidmeister left a comment

Choose a reason for hiding this comment

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

@findolor findolor mentioned this pull request Oct 15, 2024
3 tasks
@findolor
Copy link
Collaborator Author

@thedavidmeister created this PR

@thedavidmeister thedavidmeister removed their request for review October 15, 2024 14:44
Copy link
Contributor

@thedavidmeister thedavidmeister left a comment

Choose a reason for hiding this comment

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

don't merge this yet, because dependant branch is not reviewed

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.

allow for a charting entrypoint
2 participants