Skip to content

Conversation

@VegetarianOrc
Copy link
Collaborator

After reviewing the TODO comments, these ones were deemed as "won't do" currently. Happy to discuss that decision if there are any opinions.

VegetarianOrc and others added 2 commits January 9, 2026 10:08
Removes TODO comments for items marked as "won't do" during todo triage:
- Allow service to be provided as positional argument
- Error on invalid Operation instantiation
- Support dynamic Operation creation
- Rename operation variables for clarity

These items have been evaluated and decided not to be implemented.

Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
Replaces TODO comments with clear explanations that child classes
intentionally omit @nexusrpc.service at definition time because the
decorator is applied in the test function itself. This tests that
service definitions correctly inherit operations from decorated
parent classes.

Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
@VegetarianOrc VegetarianOrc requested a review from a team as a code owner January 10, 2026 00:07
def service_handler(cls: type[ServiceHandlerT]) -> type[ServiceHandlerT]: ...


# TODO(preview): allow service to be provided as positional argument?
Copy link
Contributor

Choose a reason for hiding this comment

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

My thinking here was simply that it would be cleaner / more ergonomic to say @service_handler(MyService) rather than @service_handler(service=MyService) seeing as there's only one possible argument currently and even if we add more that would always be the most important argument.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I agree that it's the most important argument. There's currently a name arg and my initial take was that it seemed fine to require being explicit. I don't feel strongly about that position so I'd be happy to get this in the todo list if the increased ergonomics are worth it.

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.

4 participants