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

Remove the renderer-skipped-selection-rendering warning #4193

Closed
Reinmar opened this issue Oct 5, 2017 · 2 comments · Fixed by ckeditor/ckeditor5-engine#1166
Closed

Remove the renderer-skipped-selection-rendering warning #4193

Reinmar opened this issue Oct 5, 2017 · 2 comments · Fixed by ckeditor/ckeditor5-engine#1166
Assignees
Labels
package:engine type:task This issue reports a chore (non-production change) and other types of "todos".
Milestone

Comments

@Reinmar
Copy link
Member

Reinmar commented Oct 5, 2017

We added it to monitor how often it's logged. Now we know, so we can remove it because the console looks bad when full of those.

@pjasiun
Copy link

pjasiun commented Oct 5, 2017

At the very beginning, we had an idea of different log levels. Maybe we should introduce it now? In fact, we already met the problem with the need to log some additional information for debugging. We introduced it as a separate plugin then. Maybe we should move this option to this plugin too, or improve the whole mechanism introducing some log levels.

@Reinmar
Copy link
Member Author

Reinmar commented Oct 5, 2017

I have mixed feelings.

On the one hand, perhaps oooone day such a warning would let you figure out some bug. Imagine – you're trying to figure out why selection isn't refreshed. You have debugging mode on (it'd need to be on by default in our tests) and you quickly realise what's that.

On the other hand, polluting our code with warnings like that which we'll use once every 3 years is not a good idea. We need to be careful when doing so because we will affect the code size.

So, if we'll find the right ballance, then ok. But this means quite a lot of work – turning on debug mode in our all manual and automated tests, adding support for 2 levels of verbosity, perhaps adding support for minifying these warnings.

Therefore, here I'd remove this warning. And I'd get back to this once we have at least one more case where we'd like to use it (unless the case you mention is really this one cause it doesn't sound).

Reinmar referenced this issue in ckeditor/ckeditor5-engine Nov 9, 2017
Other: Removed the `renderer-skipped-selection-rendering` warning since it doesn't bring any value. Closes #1158.
@mlewand mlewand transferred this issue from ckeditor/ckeditor5-engine Oct 9, 2019
@mlewand mlewand added this to the iteration 13 milestone Oct 9, 2019
@mlewand mlewand added status:confirmed type:task This issue reports a chore (non-production change) and other types of "todos". package:engine labels Oct 9, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
package:engine type:task This issue reports a chore (non-production change) and other types of "todos".
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants