HARP-13493: Make maxZoomLevel value in technique non inclusive

[germanz]

Signed-off-by: German Zargaryan german.zargaryan@here.com

Thank you for contributing to harp.gl!

Before requesting a pull request, please remember to check the following documents:

• contribution guidelines
• coding style

If you are adding new functionality we would highly appreciate if you can describe what is the capability you are adding and even better if you can add some examples. Please also remember to add tests for it.

CI Check

Our bots will check whether your PR can be directly integrated into the mainline. We have some internal integration tests running on the background, our bots will inform you of the next steps and someone from our team will take a look and help if needed!

And please do not forget to sign-off your commit! You can read more about DCO here. But, in short, you just need to use git commit -s or append --signoff when you are committing to the repo.

Happy contributing!

[codecov]

Codecov Report

Merging #2016 (0601d3b) into master (30f41eb) will increase coverage by 0.01%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##           master    #2016      +/-   ##
==========================================
+ Coverage   66.56%   66.58%   +0.01%     
==========================================
  Files         297      297              
  Lines       26341    26342       +1     
  Branches     5944     5945       +1     
==========================================
+ Hits        17535    17539       +4     
+ Misses       8806     8803       -3     

Impacted Files	Coverage Δ
...re/harp-datasource-protocol/lib/TechniqueParams.ts	72.13% <ø> (ø)
.../harp-datasource-protocol/lib/StyleSetEvaluator.ts	89.05% <100.00%> (ø)
@here/harp-mapview/lib/TileObjectsRenderer.ts	44.20% <100.00%> (ø)
@here/harp-mapview/lib/text/Placement.ts	88.37% <100.00%> (+1.03%)	⬆️

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 30f41eb...0601d3b. Read the comment docs.

[atomicsulfate]

We need to document this change in BaseTechniqueParams in @here/harp-datasource-protocol/lib/TechniqueParams.ts.

[germanz]

done

[atomicsulfate]

Please document the behaviour change as part of the next release notes:
https://github.com/heremaps/harp.gl/releases/

There's not draft of next release (0.22) so you can create a new one. Just follow the same format as the previous releases. Once done click on "Save draft", not on "Publish release".

[nzjony]

I agree with the idea of making it non-inclusive, however the naming is unfortunate, the max in maxZoomLevel is short for "maximum", which (according to google), means: "the greatest amount, extent, or intensity possible, permitted, or recorded.", i.e. the maximum always refers to an attainable value, however this changes breaks this concept.

I think a better name could be start beginZoomLevel and limitZoomLevel, where "limit" is (at least in mathematics) clear that it is a value that we get close to, but don't reach.

@atomicsulfate , @germanz , please reconsider.

[germanz]

retest this please

[germanz]

@nzjony I agree with you about the definition of the term. However, I think it is better to keep it simple and document the range [minZoomLevel, maxZoomLevel) so that the upper edge is not reachable.

It will be unfortunate to break the compatibility by renaming the property or introducing another one and cause confusion. Other Map rendering APIs also use the same terms.

[robertoraggi]

You may want to check with our Design team before going forward with this change of semantics.

[robertoraggi]

Also, shouldn't you update the semantics of all the zoom/data level constraints? e.g. DataSource.maxDisplayLevel, DataSource.maxDataLevel, MapView.maxZoomLevel, MapControls.maxZoomLevel?

[germanz]

As agreed for the time being we leave the semantic of those properties as-is, both edges inclusive.