search

ckeditor / ckeditor4-angular

Official CKEditor 4 Angular component.
Other
55 stars 32 forks source link

Enable development testing #22

Closed jacekbogdanski closed 4 years ago

jacekbogdanski commented 5 years ago

I'm not sure if we want to keep warning messages when node_modules/ckeditor package is missing and left development/release testing up to the existence of node_modules/ckeditor folder. Maybe it should be based on environment variables? I would suggest something like:

var files = [];
if ( !process.env.PRODUCTION ) {
    files = [...] // node_modules/ckeditor references
}

but it may add some unnecessary complexity. WDYT?

Honestly, I'm not sure why we are hardcoding CDN: https://github.com/ckeditor/ckeditor4-angular/blob/ddd026a18a676e7ff31958d00a2b98ccd770e629/src/ckeditor/ckeditor.component.ts#L192 which requires string update after release and adds complexity by multiple "right" ways to include CKEditor4. Nevertheless, there is no need to attach CDN by karma itself, because CKEditor module does it by default if karma didn't find dev dependency.

Closes #8

f1ames commented 4 years ago

Closing as #8 was fixed in another PR.