Skip to content

Conversation

@kerams
Copy link
Contributor

@kerams kerams commented Jan 25, 2023

Fixed the names of a couple of values as well as removed an unnecessary list creation.

The (atMostOne: ResultCollectionSettings) parameter to the function is currently not used for anything, so I could remove it if you want.

@kerams kerams marked this pull request as ready for review January 25, 2023 19:09
@kerams kerams requested a review from a team as a code owner January 25, 2023 19:09
Copy link
Contributor

@psfinaki psfinaki left a comment

Choose a reason for hiding this comment

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

Thanks for this @kerams.
As for removing the parameter - is there any potential backward compat issue @vzarytovskii?

@vzarytovskii
Copy link
Member

vzarytovskii commented Jan 26, 2023

It's internal, so we should be fine with changing it

@psfinaki psfinaki enabled auto-merge (squash) February 13, 2023 12:26
@psfinaki
Copy link
Contributor

@kerams feel free to resolve the comments and merge.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

Archived in project

Development

Successfully merging this pull request may close these issues.

4 participants