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 WSC plugin from Demo preset #56

Merged
merged 2 commits into from
Jan 12, 2021
Merged

Remove WSC plugin from Demo preset #56

merged 2 commits into from
Jan 12, 2021

Conversation

Dumluregn
Copy link
Contributor

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.

@f1ames f1ames self-requested a review January 11, 2021 15:57
@f1ames f1ames self-assigned this Jan 11, 2021
@f1ames
Copy link
Contributor

f1ames commented Jan 11, 2021

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.

I got probably the same error:

Strange, there is no pastetools plugin even though it's a dependency of pastefromword one 🤔

@f1ames
Copy link
Contributor

f1ames commented Jan 12, 2021

I checked and even 4.14.1 demo doesn't work. The demo is used on our websites so either the workflow is different there or no one reported there is something wrong for a long time 🤔

Other difference is that, demo-build-config.js file is the one generated by our online builder and has additional preset: standard property but I'm not sure if it affects anything:

Anyway, I have reached @usqr to ask how exactly this preset is used and build.

@f1ames
Copy link
Contributor

f1ames commented Jan 12, 2021

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 build.sh demo should work IMHO, since it's really confusing... The other thing is why pastetools is not added to the build even if it's a dependency of pastefromword which is there 🤔

@f1ames f1ames closed this Jan 12, 2021
@f1ames
Copy link
Contributor

f1ames commented Jan 12, 2021

Missclick... 😢

@f1ames f1ames reopened this Jan 12, 2021
Copy link
Contributor

@f1ames f1ames left a 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?

@f1ames
Copy link
Contributor

f1ames commented Jan 12, 2021

Rebased onto latest demo.

@f1ames f1ames merged commit 564f2b7 into demo Jan 12, 2021
@f1ames f1ames deleted the t/54 branch January 12, 2021 13:57
@Dumluregn
Copy link
Contributor Author

#56 (review) extracted to #58.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants