-
Notifications
You must be signed in to change notification settings - Fork 27
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 WSC plugin from Demo preset #56
Conversation
I got probably the same error: Strange, there is no |
I checked and even Other difference is that,
Anyway, I have reached @usqr to ask how exactly this preset is used and build. |
So after talking to @usqr it turns out that demo preset is build like: spawnSync( 'bash', [ 'build.sh', 'demo', 'all' ], { cwd: `${tempPath}/ckeditor4`, stdio: 'inherit' } ); which means the list of plugins doesn't make almost any difference. Still, using it like |
Missclick... 😢 |
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.
It's better to remove plugin from build config instead of using removePlugins
. I fixed it in eb53ff9.
However, currently the demo preset seems to be broken (or I can't use it thinking ) - after building the sample doesn't work, the plugins that the preset contains are odd and don't match anything. I'm not sure if I should fix this in this PR, or create a separate one.
@Dumluregn could you extract it to a separate issue, having also #56 (comment), #56 (comment) and #56 (comment) in mind?
Rebased onto latest |
#56 (review) extracted to #58. |
As stated in the issue, plugin is actually still in presets, but excluded in config using removePlugins property.
However, currently the
demo
preset seems to be broken (or I can't use it 🤔 ) - after building the sample doesn't work, the plugins that the preset contains are odd and don't match anything. I'm not sure if I should fix this in this PR, or create a separate one.Closes #54.