Skip to content

Remove ReadOnlyDictionary safety wrappers #193

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

Merged
merged 1 commit into from
Mar 29, 2024

Conversation

SergeiPavlov
Copy link
Collaborator

@SergeiPavlov SergeiPavlov commented Mar 28, 2024

Also:

  • Replace Pair<FieldInfo> by value tuple for readability

@SergeiPavlov SergeiPavlov requested a review from botinko March 28, 2024 08:09
@botinko
Copy link

botinko commented Mar 28, 2024

As far as I understand, the wrapper's overhead is quite small. I afraid that these changes may make future support of our fork more difficult for us, as changes from the DO master will increase.

@SergeiPavlov
Copy link
Collaborator Author

SergeiPavlov commented Mar 28, 2024

As far as I understand, the wrapper's overhead is quite small. I afraid that these changes may make future support of our fork more difficult for us, as changes from the DO master will increase.

They are trivial to resolve. The upstream branch changes very little last years.
There were many wrappers like this, and together the add some performance tax for us -may be small, may be not, it is better to be sure we don't pay whatever we need not

@SergeiPavlov SergeiPavlov merged commit 879d343 into master-servicetitan Mar 29, 2024
2 checks passed
@SergeiPavlov SergeiPavlov deleted the ReadOnlyWrapper branch March 29, 2024 05:21
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.

2 participants