Skip to content

Conversation

@thiemowmde
Copy link
Contributor

No description provided.

@JeroenDeDauw
Copy link
Contributor

This updated requirement kinda sucks. Is it really needed? If the only compat issue here is in the tests of this component, then you can resolve it there, without effecting users of this component that do not care about the hash.

@thiemowmde
Copy link
Contributor Author

I also was not happy with this. I had an other look and believe I found a really nice solution. What do you think?


protected function buildSerializer() {
return new SnakSerializer( new DataValueSerializer() );
return new SnakSerializer( new DataValueSerializer(), false );
Copy link

Choose a reason for hiding this comment

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

Could you name this false? Otherwise it quiet hard to read

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.

Copy link
Contributor

@JeroenDeDauw JeroenDeDauw left a comment

Choose a reason for hiding this comment

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

Yeah!

@JeroenDeDauw JeroenDeDauw merged commit e267bc2 into master Mar 16, 2017
@JeroenDeDauw JeroenDeDauw deleted the release240 branch March 16, 2017 12:13
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