Skip to content

Conversation

@blast-hardcheese
Copy link
Contributor

Why

We had a number of long-standing nice-to-haves, let's just do them

What changed

  • We discussed switching from Dict/List/Tuple/Optional the builtin types. Let's do that.
  • lambda to def for readability and maintainability
  • Fixing some slop in is_literal
  • Add # noqa to remove the need for sed in our generator scripts

Test plan

Describe what you did to test this change to a level of detail that allows your reviewer to test it

@blast-hardcheese blast-hardcheese requested a review from a team as a code owner March 14, 2025 21:00
@blast-hardcheese blast-hardcheese requested review from ryantm and removed request for a team March 14, 2025 21:00
@blast-hardcheese blast-hardcheese force-pushed the dstewart/chore/quality branch 4 times, most recently from 965942d to ebb90c7 Compare March 17, 2025 20:55
@blast-hardcheese blast-hardcheese enabled auto-merge (squash) March 17, 2025 23:29
@blast-hardcheese blast-hardcheese merged commit b6215c9 into main Mar 17, 2025
3 checks passed
@blast-hardcheese blast-hardcheese deleted the dstewart/chore/quality branch March 17, 2025 23:30
blast-hardcheese added a commit that referenced this pull request Mar 18, 2025
Why
===

Introduced in #142, `NotRequired[A] | None` doesn't typecheck, but we
didn't have any codegen tests that exhausted that path.

What changed
============

Move the statically rendered `${render(...)} | None` into
`${render(UnionTypeExpr([..., NoneTypeExpr()]))}` so the type renderer
knows about and can properly unify/deduplicate/render the structured
union.

Test plan
=========

TODO: Add a test that makes it so we don't regress here in the future.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants