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

Conversation

@mlewand
Copy link
Contributor

@mlewand mlewand commented Feb 26, 2020

Suggested merge commit message (convention)

Feature: Added support for initializing Collection items using a constructor. Closes ckeditor/ckeditor5#6319.


Additional information

This PR was mostly based on #309 with some notable changes:

  • Reverted collection-add-invalid-id and collection-add-item-already-exists error interfaces back to what we have on the master branch (see current vs originally proposed). I don't think there's a strong reason to change this API.
  • add() remains unchanged with exception of reusing _getItemIdBeforeAdding().
  • Added API docs.
  • Added couple more test cases: generates id for items that doesn\'t have it, throws an error when invalid item key is provided, throws an error when items have a duplicated key.

I had some doubts:

Once this PR is merged, the master branch should get merged to the #309 PR so that the diff will get smaller and easier to review.

There's a subpr: ckeditor/ckeditor5-ui#545

@coveralls
Copy link

coveralls commented Feb 26, 2020

Coverage Status

Coverage remained the same at 100.0% when pulling 32866ae on i/6319 into b148e9c on master.

@mlewand mlewand marked this pull request as ready for review February 27, 2020 08:24
@mlewand mlewand requested a review from oleq February 27, 2020 08:25
@mlewand mlewand requested a review from oleq March 11, 2020 13:56
@oleq
Copy link
Member

oleq commented Mar 12, 2020

Let's merge it ASAP in the upcoming iteration.

@oleq oleq merged commit 8846e66 into master Mar 23, 2020
@oleq oleq deleted the i/6319 branch March 23, 2020 10:40
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Collection constructor should allow to set initial values

5 participants