-
Notifications
You must be signed in to change notification settings - Fork 111
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
Improve readme and docs and point out that we support only Angular >= 5.0.0 #95
Comments
I think the problem was with testing the integration with the previous versions of Angular. Without that, since we're talking about breaking semver changes, we have no guarantee that it will work. So, to safely state that we support e.g. 4+ we need to test with all of those versions. It's doable, but needs to be done continuously. WDYT @ma2ciek? |
Actually, we support only Angular 5+ because of a backward-incompatible format of Angular meta-data produced by the builder. And we took this name only to ensure that nobody will try to use it with old AngularJS. I've checked it some time ago - #41. Probably is doable to provide a package for Angular@4 and lower versions but we'd have to maintain a few branches and IDK how we'd have to make releases. |
It's because we use the package.json for the development, we can't use it to publish on NPM. That's why we have a separate package.json for NPM: https://github.com/ckeditor/ckeditor5-angular/blob/master/src/ckeditor/package.json and it clearly states what are the versions that we support, so anybody trying to install this package from the NPM will be warned if he doesn't installed other packages at correct versions. And having two package.json files is common for Angular packages. |
We can improve readmes but IDK if we can improve more. |
Okay so it's much more clear for me now. Some note in README and the documentation explaining it a bit better would do the trick I guess. |
Maybe we can also create a "fake comment" in the main package.json file to warn users that this package.json file won't be served by NPM. |
I don't know if it won't be easier if we replace all |
As something like |
Good question. I'd get rid of the version number in any official titles/headings anywhere. The information about compatibility with Angular versions should be only inside documentation/readme, not in the title (and not in the headers of the documentation, because of the issue you pointed out). The lowest version that we support will change possibly relatively frequently, because supporting Angular version that is no longer supported by the vendor does not look good too, AngularJS popularity is dropping and less and less people are using it in new projects so if accidentally someone will be trying to use our integration with AngularJS, it would just mean he did not read the documentation. |
Other: Removed usages of `Angular 2+` from code and readmes; added information about supported Angular versions. Closes #95.
I updated the docs in ckeditor/ckeditor5@a45fb7b If you feel that we need to add the information from https://github.com/ckeditor/ckeditor5-angular/blob/master/README.md#supported-angular-versions to the docs, we can also do it. |
I'll add there a short note too. |
The documentation states that the integration works with Angular 2+. Recently we received a support request if the integration works with Angular 6 despite that.
The question was asked because the dependencies state
^7.0.1
instead of something like>=2.4.0
(or whatever version was confirmed that works fine). So indeed the current way the dependencies are set seem to be confusing and problematic.The text was updated successfully, but these errors were encountered: