Skip to content
This repository has been archived by the owner on Jun 26, 2020. It is now read-only.

Added iterator interface to the Config class. #316

Merged
merged 3 commits into from
Dec 16, 2019
Merged

Added iterator interface to the Config class. #316

merged 3 commits into from
Dec 16, 2019

Conversation

oskarwrobel
Copy link
Contributor

@oskarwrobel oskarwrobel commented Dec 3, 2019

Suggested merge commit message (convention)

Feature: Added iterator interface to the Config class. Part of ckeditor/ckeditor5#5891.


Additional information

Required by the ckeditor/ckeditor5-core#204. Editor#config needs to be extended by the Context#config.

@coveralls
Copy link

Coverage Status

Coverage remained the same at 100.0% when pulling f9bd16e on context into 40a5ad7 on master.

src/config.js Outdated
*
* @returns {Iterable.<String>}
*/
[ Symbol.iterator ]() {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If it's iterating over names I'd create it under Config#names() or Config#keys() depending on how we call this thing. Similar to Map#keys() vs Map#iterator where the former iterates over prop names and the other over all entries.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done. I added a Config#names() method.

Copy link
Member

@Reinmar Reinmar left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As commented.

@oskarwrobel oskarwrobel requested a review from Reinmar December 6, 2019 10:56
@scofalik scofalik merged commit 1fdf2f1 into master Dec 16, 2019
@scofalik scofalik deleted the context branch December 16, 2019 09:41
# for free to subscribe to this conversation on GitHub. Already have an account? #.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants