search

wmde / DataValuesJavaScript

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

Don't register the TimeParser #28

Closed adrianheine closed 10 years ago

adrianheine commented 10 years ago

Wikibase wants to use its own API-based parsing.

We might consider removing the TimeParsers, too. Also we should consider in which release this change should go (0.3.2 or 0.4.0?). See also Wikibase change.

tobijat commented 10 years ago

Please always update the release notes section in the readme along with the change. Please state there if it is a breaking change to the currently used versions of the dependent components. If so, then please increment the version number in the readme to 0.4 (dev) and in DataValuesJavascript.php to 0.4 (alpha).

adrianheine commented 10 years ago

As I stated in the pull request, it is unclear in which version this should go; that's why I left out the README entry.

JeroenDeDauw commented 10 years ago

0.4 - this is a breaking change, which you cannot do in a patch rel, even when still having a dev version (ie starting with 0.)

adrianheine commented 10 years ago

As a sidenote, theoretically we can do whatever we want to: »Major version zero (0.y.z) is for initial development. Anything may change at any time. The public API should not be considered stable.« In practice, we should stick to the convention we use.

I think you are right that this is a breaking change, so it's gonna be 0.4.0.

JeroenDeDauw commented 10 years ago

Theoretically yes - not practically - wb will happily pull the breaking version in if you go for 0.3.x.

snaterlicious commented 10 years ago

We should remove the mw.ext.* files completely. It is confusing why some parsers/formatters are defined and registered in DataValuesJavaScript while others are defined but not registered. Registering default parsers/formatters appears to be out of scope for this module - all MW related initialization should be taken care of in MW context / Wikibase.

JeroenDeDauw commented 10 years ago

Registering default parsers/formatters appears to be out of scope for this module

Yeah, libraries should not have global state.

all MW related initialization should be taken care of in MW context / Wikibase.

Exactly. Happy to see we are making progress in this area :)

adrianheine commented 10 years ago

I think being able to override parsers/formatters/validators or specify preference or something like that would be reasonable, too.

JeroenDeDauw commented 10 years ago

I think being able to override parsers/formatters/validators or specify preference or something like that would be reasonable, too.

As long as we do not have this as global state in the library. It can have multiple users that want to have a different "default". If a default is provided, then doing so as a function that returns it is fine. Providing it as variable seems risky, as then users might go use it as actual global state by modifying it and using it though their code, rather than fetching it once, modifying that copy to their desire, and managing it further themselves.