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

Runtime Observable Settings #184

Merged
merged 4 commits into from
Mar 25, 2024

Conversation

psuzn
Copy link

@psuzn psuzn commented Feb 25, 2024

This adds support for the wrapper around the normal setting (#155 ) to make it runtime observable.

@russhwolf I have named it runtime-observable but we can rename it if there is a better name.

I also have added the checkObservable flag on the converter extension function, so one can just use the no-arg initializer and use the converter to make it observable. In some platforms, the instance returned by no-arg is already observable; in that case, the converter doesn't add any wrapper.

@russhwolf
Copy link
Owner

Thanks! I'm traveling right now so I probably won't get a chance to review this closely for a week or so.

I'm still not sure how I feel on the naming, but will probably merge it with what you have and change it before I put out a release with it.

Speaking of the next release, I already have a 1.2 branch with some other pending updates. Do you think you can rebase against that branch? If not, I can do it when I'm back. I'm guessing the only major change needed will be the gradle files because I converted the old buildsrc code to use script plugins.

@russhwolf
Copy link
Owner

To fix the CI failure, run the apiDump gradle task from the new module and push the api files it generates to this branch.

@psuzn psuzn changed the base branch from main to 1.2 February 26, 2024 02:55
@psuzn psuzn force-pushed the sp/runtime-observable-settings branch from 109671d to 8da35e3 Compare February 26, 2024 03:51
@psuzn
Copy link
Author

psuzn commented Mar 20, 2024

@russhwolf, any update on this?

Copy link
Owner

@russhwolf russhwolf left a comment

Choose a reason for hiding this comment

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

Sorry to leave this sitting. I've had less time over the last couple weeks than I'd hoped. I plan to merge this soon and probably make a couple tweaks on top of it, but had a couple clarifying questions first.

@russhwolf russhwolf merged commit ab87526 into russhwolf:1.2 Mar 25, 2024
3 checks passed
@russhwolf
Copy link
Owner

Thanks!

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.

None yet

2 participants