Skip to content
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

Fix issue with set readonly throwing an exception #99

Merged
merged 1 commit into from
Jul 26, 2023

Conversation

arontsang
Copy link
Contributor

No description provided.

@arontsang
Copy link
Contributor Author

fixes #83

@nbudin
Copy link
Owner

nbudin commented Jul 26, 2023

I have somewhat mixed feelings about this, because it feels like making the update behavior dependent on readOnly is non-obvious and could backfire in surprising ways. But, on the other hand, the current behavior is also surprising, so I think this is a good idea and I'll merge it.

This is sort of in the gray area between "breaking" and "non-breaking" change. My thought is that it's unlikely to break any actual use of this API, so I'm going to make this a minor version bump.

Thanks for the PR!

@nbudin nbudin merged commit f841446 into nbudin:main Jul 26, 2023
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