Skip to content

Conversation

rrbe
Copy link
Contributor

@rrbe rrbe commented Aug 12, 2020

See #23

Copy link
Owner

@philipstanislaus philipstanislaus left a comment

Choose a reason for hiding this comment

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

Thanks, left a few comments.

I feel that we should not merge the supplied array with the default, but replace it. Otherwise there is no fine grained control when to throw. What do you think?

@rrbe
Copy link
Contributor Author

rrbe commented Aug 13, 2020

Thanks, left a few comments.

I feel that we should not merge the supplied array with the default, but replace it. Otherwise there is no fine grained control when to throw. What do you think?

These modifications are completed, and I also added test case about the rootParentId options replacement, please check it and see what else needs to be modified

Copy link
Owner

@philipstanislaus philipstanislaus left a comment

Choose a reason for hiding this comment

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

Thanks, just one more note, please see the comments.

Copy link
Owner

@philipstanislaus philipstanislaus left a comment

Choose a reason for hiding this comment

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

@rrbe sorry for the very late feedback, I just remembered there were some issues/PR still open here.

I have thought about the architecture a bit again and changed the data type for rootParentIds to an object for better performance.

This should be fine now – I will merge it shortly.

Many thanks for your contribution and sorry again for the delay!

@philipstanislaus philipstanislaus merged commit 4fe9dc2 into philipstanislaus:master Feb 2, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

2 participants