Skip to content

Conversation

@thiemowmde
Copy link
Contributor

@thiemowmde thiemowmde commented Feb 13, 2017

I suggest to merge everything in https://github.com/wmde/WikibaseDataModelSerialization/milestone/7 (#206, #207 and #208) and tag an early release 2.3.0.

Then merge #212 and tag a braking 3.0.0 release.

Bug: T157959

@thiemowmde thiemowmde added this to the 2.3.0 milestone Feb 13, 2017
@manicki
Copy link
Member

manicki commented Feb 13, 2017

#206 is in, I am not sure though about the micro-optimization ones #207 and #208.
I have no problem with them (I am not sure how much we win with those but they make the code somewhat faster, yes) but it would be nice to have a nod from @JeroenDeDauw if he is fine with them, or if he would like to see hard numbers on how much impact they have on whole Wikibase etc.
I am probably going to merge those if there is nothing happening out there in some more time, though.

@JeroenDeDauw
Copy link
Contributor

I'm very skeptical of those changes. However I don't think merging them really makes a difference either way. This is very local and not significant compared to the real issues in the wider Wikibase codebase. What I do think would be harmful is continuing with such inlining and doing so based on wishful thinking.

@manicki
Copy link
Member

manicki commented Feb 14, 2017

Thanks @JeroenDeDauw, I share scepticism.
I believe starting from version 2.3/3.0 of this component we should try to provide hard data to justify one or other optimization.

@manicki manicki merged commit b5c2666 into master Feb 15, 2017
@manicki manicki deleted the release230Now branch February 15, 2017 10:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

4 participants