Remove several methods

[Ladsgroup]

• GlobeCoordinate. getDecimal has not been used since a50a48e
• QuantityValue.get(UpperBound|LowerBound|Amount) has not been used
  since fadb3ab

[lucaswerkmeister]

Okay, I have to admit, removing the QuantityValue methods feels strange to me. What are we even using this class for if we don’t use those getters at all? (And I suspect the answer is, we only construct instances of the class and then serialize them to JSON, without processing them very much.)

Anyways, couldn’t getUnit be removed then as well? As far as I can tell it’s not used either.

[Ladsgroup]

Anyways, couldn’t getUnit be removed then as well? As far as I can tell it’s not used either.

It's used in QuantityInput.js

[lucaswerkmeister]

Anyways, couldn’t getUnit be removed then as well? As far as I can tell it’s not used either.

It's used in QuantityInput.js

Which file is that? I can’t find it in this repository or in Wikibase.

[Ladsgroup]

Anyways, couldn’t getUnit be removed then as well? As far as I can tell it’s not used either.

It's used in QuantityInput.js

Which file is that? I can’t find it in this repository or in Wikibase.

https://gerrit.wikimedia.org/r/plugins/gitiles/data-values/value-view/+/refs/heads/master/src/experts/QuantityInput.js

[lucaswerkmeister]

Okay, thanks.

I’m still not sure about those QuantityValue removals, though. This feels different than a lot of the other unused code removals – this isn’t much code, at least not after minification, and the class feels incomplete without those getters (especially if getUnit() is left behind because that one happens to be used at the moment – but if we want to use any of the other getters in the future, we’ll first have to publish a new release of this library to add them back, or use the internal members directly).

(No objections from me to removing GlobeCoordinate.getDecimal, by the way.)

[Ladsgroup]

I’m still not sure about those QuantityValue removals, though. This feels different than a lot of the other unused code removals – this isn’t much code, at least not after minification, and the class feels incomplete without those getters (especially if getUnit() is left behind because that one happens to be used at the moment – but if we want to use any of the other getters in the future, we’ll first have to publish a new release of this library to add them back, or use the internal members directly).

I agree with you and would have not done it if it was a couple of years ago but now the long term plan is to burn all of this code in fire and replace everything with a more modern code.
Regarding the release: These codes are submodules currently, I just bump it to HEAD (no need to release anything).

[lucaswerkmeister]

Hm, okay.

[lucaswerkmeister]

Though that makes me wonder if we should add a “0.11.0 (dev)” to the README with this, even if we don’t bump the version in package.json yet?

[Ladsgroup]

Though that makes me wonder if we should add a “0.11.0 (dev)” to the README with this, even if we don’t bump the version in package.json yet?

I was like "I'm sure I wrote this" and then I looked and realized I haven't "git add"ed it.