-
Notifications
You must be signed in to change notification settings - Fork 836
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
chore: deprecate spell checker service #2255
Conversation
…ovider, ignore all warnings
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you for taking the time to deprecate this feature. For now these changes are more than enough because SpellCheckerService
is in a very early stage. And as you mentioned, it's best to release these new features only when they are stable (which in this case is the least likely, since I couldn't provide a more compact solution that would fit the needs of the developers).
For now I'll take the time to start fixing problems instead of adding new features, since so many new things really add more complexity to the project.
Thanks. Users should still be able to use simple_spell_checker even after the breaking change. |
I think that's fine, but I'm planning to break changes anyway because i need to improve the current implementation of it, which is not only slow, but cumbersome and sometimes doesn't even fulfill its basic function. It will take some time anyway. Thanks for taking the time and effort. |
If you're going to introduce breaking changes and in case you have the time, you probably want to separate support of each language into its own package, use melos to automate the publishing since it's can be time-consuming. There are other things to consider such as web support, how will these files will be supported? especially for the web, don't know how Flutter Web loads files but shouldn't be more memory performance efficient when loading them if they were file assets instead? Do they slow down the app loading on the web if deployed? Of course, it might not be a priority until now or you might not have the time at the moment for those changes. |
Thanks a lot for the suggestion, I'll probably use it. And I guess since you mentioned making a package for each language, it gave me an idea of how I'll do this.
Apparently the website is able to load the dictionary files, but since they are large (normally, 80 MB is too much for such a small feature) it takes longer for a page to load.
Yes, for now I will keep the package the same while I create an implementation that is more in line with what is required and that is used by developers. I say this because I have noticed that some people make forks to disable all other languages and leave only the one they will use. This gives me the impression that, as you mention, having languages in separate packages would allow all developers to have only the languages they are interested in instead of having everything (which they will probably only use one or two of those dictionaries) Thank you very much for your recommendations. I really appreciate the help. |
Can we please merge some of PRs before making more? |
* chore: deprecate SpellCheckerService * docs(readme): update docs to reflect the change * chore: deprecate DefaultSpellCheckerService and SpellCheckerServiceProvider, ignore all warnings
* chore: deprecate SpellCheckerService * docs(readme): update docs to reflect the change * chore: deprecate DefaultSpellCheckerService and SpellCheckerServiceProvider, ignore all warnings
Description
Deprecate
SpellCheckerService
as a breaking change will arrive soon.Make it clear to the users that the
SpellCheckerService
should not be used as breaking change will be arriving soon, we provided an implementation in the example only (with the usage of simple_spell_checker) and mentioned that this is not a feature implemented in the project, which doesn't mean we can remove theSpellCheckerService
right away. We should at least give the users a few weeks (which is not enough) if we're going to introduce this change in a minor breaking change release (10.7.0
).Should also be mentioned that, will cause issues for most projects that use it since pub.dev (pub_semver) expects the version to follow the same format and guidelines as semver. Some projects use tools to automate the process of updating the dependencies to the latest non-breaking release and then publishing a new version (sometimes).
flutter pub upgrade --major-versions
will update them even if breaking changes are introduced.flutter pub upgrade
will update to the latest non-breaking release. It assumes that10.7.0
is not a breaking change.Temporarily removed the docs as it can be used to indicate that this is a stable feature. We do want to limit the damage that can be caused by fixing #2246.
Related Issues
Type of Change
Suggestions
I highly suggest marking new features (including #2230) as experimental, marking them as experimental doesn't necessarily mean the feature is likely to be broken or incomplete but more like this is not stable and can be changed in a way that users will also have to update their code in order to use newer versions. This gives us the opportunity to fix issues without having to worry about releasing a new major breaking release.
We should be more direct and straight with the users to explain the breaking changes if having minor breaking changes such as updating the name of something (or correct misspellings, example without breaking change for internal use only class) in minor versions is acceptable (while mentioning this in
README
)?In all cases we should be more cautious about adding new APIs and should be at least marked as experimental, I prefer that we don't add any new features for the next year (or even more) once we fix all issues and code quality issues. This will be a long way and we don't want to give promises we can't keep up to. So no guarantee that this will be worth it.
But having more incomplete features (which is very often) with more issues, and even more code quality issues doesn't seem to be a solution at all, if you disagree, you might see example projects are moving away from the project. Fixing this is simple, which doesn't mean it's simple to execute.
We had more discussion about this in #2235.