Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

@pavelvasev merge: Component context optimization.. maybe we do not need it? #249

Merged
merged 1 commit into from
Jun 10, 2016

Conversation

ChALkeR
Copy link
Member

@ChALkeR ChALkeR commented Jun 9, 2016

if we create context for every createComponent call, maybe it is too
heavy... So we introduce option to provide context in createObject
method which overrides it's component context.

This merges 871fbf0, ref: #32.

/cc @pavelvasev @Plaristote

@ChALkeR ChALkeR added the merge label Jun 9, 2016
@ChALkeR ChALkeR mentioned this pull request Jun 9, 2016
62 tasks
@Plaristote
Copy link
Member

Well, it LGTM.
But there's not much left of the original commit, and I'm wondering if the message is still appropriate for the changes that are made in here. The component cache is gone, the only thing added is the ability to overload the context that createObject usually uses, right ?

@ChALkeR
Copy link
Member Author

ChALkeR commented Jun 10, 2016

@Plaristote Thanks! I will amed the commit message.

@ChALkeR
Copy link
Member Author

ChALkeR commented Jun 10, 2016

Ok, everything is good here, landing.

@ChALkeR
Copy link
Member Author

ChALkeR commented Jun 10, 2016

For reference, this removes usage of non-standard second parameter of Qt.createComponent, which was removed in ff4002c.

That non-standard parameter was removed in
ff4002c

This introduces a non-standard parameter for component.createObject(),
but that one doesn't intersect with the standard parameters list.

PR-URL: #249
Reviewed-By: Michael Martin Moro <michael@unetresgrossebite.com>
@ChALkeR ChALkeR merged commit 2c7b240 into master Jun 10, 2016
@ChALkeR ChALkeR deleted the merge-pavelvasev-coco-opt branch June 13, 2016 20:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants