Skip to content

Conversation

@sid-rl
Copy link
Contributor

@sid-rl sid-rl commented Dec 8, 2025

No description provided.

@sid-rl sid-rl force-pushed the siddarth/scenario-builder-sdk branch from 4ffbf98 to 980a9ff Compare December 8, 2025 23:42
@sid-rl sid-rl force-pushed the siddarth/scenario-builder-sdk branch 3 times, most recently from 341c74d to f1b1ca6 Compare December 9, 2025 21:13
Copy link
Contributor

@james-rl james-rl left a comment

Choose a reason for hiding this comment

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

Some feedback for helpful docstring messages.

@sid-rl sid-rl force-pushed the siddarth/scenario-builder-sdk branch from 177dbda to 034399b Compare December 11, 2025 00:46
@sid-rl sid-rl changed the title WIP feat(scenarios): add scenario builder to sdk feat(scenarios): add scenario builder to sdk Dec 11, 2025
Copy link
Contributor

@james-rl james-rl left a comment

Choose a reason for hiding this comment

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

This looks very close to being ready. The only change I think you need to make is to have the builder use a build() method and then to call push() on the constructed object.

Just to be clear, this is not a matter of correctness, it's about adhering to conventions and user expectations.

"validation_type": self._validation_type,
}

def preview(self) -> ScenarioPreview:
Copy link
Contributor

Choose a reason for hiding this comment

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

I like this approach

@sid-rl sid-rl requested a review from james-rl December 11, 2025 02:07
Copy link
Contributor

@james-rl james-rl 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 to me, nice work!

@sid-rl sid-rl merged commit 2d41a15 into main Dec 12, 2025
7 checks passed
@sid-rl sid-rl deleted the siddarth/scenario-builder-sdk branch December 12, 2025 18:17
@stainless-app stainless-app bot mentioned this pull request Dec 12, 2025
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.

3 participants