wmde / DataValuesJavaScript

JavaScript implementations of all basic DataValue classes, associated parsers and formatters
http://wikiba.se
Other
5 stars 1 forks source link

Implement UnDeserializableValue::newFromJSON and toJSON #78

Closed filbertkm closed 9 years ago

filbertkm commented 9 years ago

made the error attribute of the value be a string, with the error reason, for more consistency with the backend implementation and for better ease of use.

also fixed more of the wording and variable names from "ununserializable" to "undeserializable", for consistency purposes.

Bug: T99231

thiemowmde commented 9 years ago

If you want we can merge this and fix the tiny issues later. Please tell me.

filbertkm commented 9 years ago

I think we need to go back to the way I ordered the params before. UnDeserializableValue inherits from dv.DataValue which has a specific order. I think it's best to be consistent with that, even if it's different that in php.

thiemowmde commented 9 years ago

What do you mean with "specific order [in] dv.DataValue"?

filbertkm commented 9 years ago

@thiemowmde suppose the order doesn't totally matter, but in SnakDeserializer we have:

this.newDataValue = function( dataValueType, data )

it is independent though from the order we order the params here, and suppose we could change. i just prefer the order to be same as there.

thiemowmde commented 9 years ago

For reference:

filbertkm commented 9 years ago

gone back again and made the order "value, type, error" throughout the value class and am adjusting the SnakSerializer. ( i really don't care, but suppose it's consistent with php is a good reason)

filbertkm commented 9 years ago

and squashed the commits

thiemowmde commented 9 years ago

Thanks. +2 when the tests succeed.

filbertkm commented 9 years ago

https://phabricator.wikimedia.org/T100985

filbertkm commented 9 years ago

https://phabricator.wikimedia.org/T96892 is the correct task