Skip to content

Conversation

@jlamberts-yobi
Copy link
Contributor

Adds support for most annotations. For our use case, we actually really just need field-level annotations, but I took a look at the issues and saw others were interested in broader annotations in #278, so figured I'd include those as well.

Specifically:

  • Struct-level annotations get a __thrift_annotations__ attr, with struct field annotations under the __thrift_field_annotations__ attr
  • Enum-level annotations get a __thrift_annotations__ attr, with item annotations under __thrift_item_annotations__
  • Service-level annotations get a __thrift_annotations__ attr, with function annotations under __thrift_function_annotations__
  • The top-level module gets lazy typedef alias and const annotations under __thrift_typedef_annotations__ and __thrift_const_annotations__, respectively

Note that this PR does not add support for annotations on the types, mostly because that would touch a lot more code: since types are represented by ints, we can't easily add metadata (like annotations).

So we parse the "foo=bar" annotation, but not the unicode.encoding annotation in typedef string ( unicode.encoding = "UTF-16" ) non_latin_string (foo="bar").

@jlamberts-yobi jlamberts-yobi marked this pull request as ready for review January 7, 2026 00:52
@aisk aisk self-requested a review January 7, 2026 02:15
@aisk
Copy link
Member

aisk commented Jan 8, 2026

Hi @jlamberts-yobi, thank you for your contribution. This feature and the code quality look very good at first glance.

However, I’m quite busy these days, so I can’t finish the review right now. I plan to complete it this weekend or early next week, so please be patient.

And let’s also see the review comments from other people during this period.

@jlamberts-yobi
Copy link
Contributor Author

Hi @jlamberts-yobi, thank you for your contribution. This feature and the code quality look very good at first glance.

However, I’m quite busy these days, so I can’t finish the review right now. I plan to complete it this weekend or early next week, so please be patient.

And let’s also see the review comments from other people during this period.

Awesome, sounds good!

@jlamberts-yobi
Copy link
Contributor Author

Hi @jlamberts-yobi, thank you for your contribution. This feature and the code quality look very good at first glance.

However, I’m quite busy these days, so I can’t finish the review right now. I plan to complete it this weekend or early next week, so please be patient.

And let’s also see the review comments from other people during this period.

Hey @aisk , just following up to see if you had any other thoughts or feedback on this change.

@cocolato
Copy link
Collaborator

Thanks! LGTM.

Remove trailing space

Co-authored-by: AN Long <aisk@users.noreply.github.com>
@cocolato cocolato merged commit cb8f8ba into Thriftpy:master Jan 21, 2026
25 checks passed
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