Skip to content
This repository has been archived by the owner on Jul 1, 2023. It is now read-only.

Switch to a singleton store #45

Merged
merged 2 commits into from
Sep 30, 2021
Merged

Switch to a singleton store #45

merged 2 commits into from
Sep 30, 2021

Conversation

liamappelbe
Copy link
Contributor

@liamappelbe liamappelbe commented Sep 28, 2021

This is one of the feature requests from #38

This change should be transparent to users at this stage, but will enable future features like sharing memory or functions between modules.

Copy link
Contributor

@kevmoo kevmoo left a comment

Choose a reason for hiding this comment

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

Would you expand on the change here? Why is it done?

Also, does this change the public API?

If so, we need to bump the pubspec version and add a changelog entry

@liamappelbe
Copy link
Contributor Author

Would you expand on the change here? Why is it done?

Also, does this change the public API?

If so, we need to bump the pubspec version and add a changelog entry

Expanded the description a bit. This change doesn't affect the API at all.

@kevmoo
Copy link
Contributor

kevmoo commented Sep 28, 2021

Is it worth a changelog entry? It changes behavior, right?

Should we bump to v0.2.0?

@liamappelbe
Copy link
Contributor Author

It doesn't change user facing behavior. Users don't have access to the stores.

@kevmoo
Copy link
Contributor

kevmoo commented Sep 29, 2021

@liamappelbe – then why are folks asking for the change? 😄

This change does SOMETHING. There are reasons folks want the change, right?

Can't we put that in the changelog?

@liamappelbe
Copy link
Contributor Author

It doesn't do anything by itself, but it will enable me to make some user facing changes that will go in the changelog. If you prefer, I can hold off submitting this until I have some of those changes to go with it.

@kevmoo
Copy link
Contributor

kevmoo commented Sep 29, 2021

It doesn't do anything by itself, but it will enable me to make some user facing changes that will go in the changelog. If you prefer, I can hold off submitting this until I have some of those changes to go with it.

Oh! Well...okay then. 😄

@kevmoo
Copy link
Contributor

kevmoo commented Sep 29, 2021

@liamappelbe
Copy link
Contributor Author

FYI: https://github.com/dart-lang/sdk/wiki/External-Package-Maintenance#making-a-change

Ok, from reading that, I think it probably does make sense to add a -dev version to the changelog, and group this change with the features that depend on it into a new minor version. Still getting used to this workflow 🙃

@kevmoo
Copy link
Contributor

kevmoo commented Sep 29, 2021

You can still land this change – but at least with a version bump and a -dev

@liamappelbe liamappelbe merged commit 48ce308 into main Sep 30, 2021
@liamappelbe liamappelbe deleted the store branch September 30, 2021 18:34
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants