Problem/Motivation

Our build-config.js deviates from the default CKeditor one in formatting and ordering which makes it difficult to make out the substantive differences, most importantly added and removed plugins.

Proposed resolution

Update build-config.js to make it resemble the upstream in terms of formatting and ordering.

Remaining tasks

User interface changes

None.

API changes

None.

Data model changes

None.

CommentFileSizeAuthor
#5 2850642-5.patch2.72 KBWim Leers
#2 diff-upstream-to-drupal--after-patch.txt3.03 KBtstoeckler
#2 diff-upstream-to-drupal--before-patch.txt3.85 KBtstoeckler
#2 2850642-2.patch2.53 KBtstoeckler
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

tstoeckler created an issue. See original summary.

tstoeckler’s picture

tstoeckler
he/him
Primary language German
Location Essen, Germany
CreditAttribution: tstoeckler as a volunteer commented
Status: Active » Needs review
FileSize
2850642-2.patch2.53 KB
diff-upstream-to-drupal--before-patch.txt3.85 KB
diff-upstream-to-drupal--after-patch.txt3.03 KB

Here we go. Attaching a diff to the upstream build-config.js (4.6.0 tag) to see the rationale for this issue.

The patch doesn't actually change anything substantially, although because of the formatting changes and reordering that needs to be verified manually.

Wim Leers’s picture

Wim Leers
Location Ghent 🇧🇪🇪🇺
CreditAttribution: Wim Leers at Acquia commented
Title: Reformat Ckeditor build-config.js to match upstream » Reformat CKEditor build-config.js to match upstream
Priority: Normal » Minor
Status: Needs review » Reviewed & tested by the community
Issue tags: +maintainability

AFAIK this is because they modified their build-config.js over time :)

+1 for making this consistent, hence more diffable & maintainable! Thanks @tstoeckler!

xjm’s picture

xjm
she/her
Primary language English
CreditAttribution: xjm at Acquia commented
Version: 8.4.x-dev » 8.3.x-dev
Status: Reviewed & tested by the community » Needs work
Issue tags: +Needs reroll, +rc target

Since the maintainability between core branches is also affected, and since this basically just ordering and formatting for the same data, I wanted to also backport it to 8.3.x to keep core in line. Thanks @Wim Leers for the subsystem maintainer review and thanks @tstoeckler for both the before and after diffs.

However, it no longer applies, probably because of the recent vendor update for CKEditor?

Marking as an RC target also given the maintainability concern.

Wim Leers’s picture

Wim Leers
Location Ghent 🇧🇪🇪🇺
CreditAttribution: Wim Leers at Acquia commented
Status: Needs work » Reviewed & tested by the community
Issue tags: -Needs reroll
FileSize
2850642-5.patch2.72 KB

However, it no longer applies, probably because of the recent vendor update for CKEditor?

Correct.

Rerolled. Did a new CKEditor build to confirm that this results in exactly the same build. It does.

  • xjm committed 8f3b710 on 8.3.x 
    Issue #2850642 by tstoeckler, Wim Leers: Reformat CKEditor build-config....
xjm’s picture

xjm
she/her
Primary language English
CreditAttribution: xjm at Acquia commented
Status: Reviewed & tested by the community » Patch (to be ported)
Issue tags: -rc target

Thanks @Wim Leers. Committed to 8.4.x and backported to 8.3.x.

xjm’s picture

xjm
she/her
Primary language English
CreditAttribution: xjm at Acquia commented
Status: Patch (to be ported) » Fixed
xjm’s picture

xjm
she/her
Primary language English
CreditAttribution: xjm at Acquia commented
Priority: Minor » Normal

Also this is a normal because it has actual benefit.

  • xjm committed ced2066 on 8.4.x 
    Issue #2850642 by tstoeckler, Wim Leers: Reformat CKEditor build-config....

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.