-
Notifications
You must be signed in to change notification settings - Fork 79
chore(shell-api): combine common Cursor + AbstractCursor code #809
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
Conversation
52b9cc4
to
9ae519b
Compare
packages/i18n/src/locales/en_US.ts
Outdated
} | ||
}, | ||
}, | ||
AbstractCursor: { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why is this necessary here? I don't think we want to make this class visible to the user in any way or?
Can we rather exclude this one with some annotation on the AbstractCursor
class, something like @abstract
or @nohelp
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
And of course i've tagged random users ... 🤦
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we rather exclude this one with some annotation on the AbstractCursor class, something like
@abstract
or@nohelp
?
Yes, let me try that (I think @gribnoysup’s #784 also has the potential to make this kind of thing much easier in the future)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@mcasimir Okay, done :) Feel free to take another look
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, totally can take inheritance chains while ignoring those abstract classes into account when generating metadata
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nice!
Create an `AbstractCursor` class (named like the driver equivalent) as a common base class for `Cursor` and `AggregationCursor` to reduce code duplication.
This helps with more code deduplication, because it can make wrappers go away that were only needed to re-specify the return type in a subclass.
898e8f3
to
d819c29
Compare
chore(shell-api): combine common Cursor + AbstractCursor code
Create an
AbstractCursor
class (named like the driver equivalent)as a common base class for
Cursor
andAggregationCursor
to reduce code duplication.
chore(shell-api): allow
@returnType('this')
This helps with more code deduplication, because it can make wrappers
go away that were only needed to re-specify the return type
in a subclass.