Rework remaining modules with multiple provides #6205

merged 10 commits into from Dec 7, 2016


None yet

2 participants

tschaub commented Dec 4, 2016 edited

After #6204, this will give us one provide per module.

There is inconsistency here that isn't all that great - but I tried to make pragmatic choices when moving enums to dedicated modules or tacking them on to existing constructors.

So, for example, we have a dedicated module for the ol.geom.GeometryType enum (instead of a Type enum on ol.geom.Geometry) because

  1. it is used in many places throughout the library (and perhaps externally), and
  2. we already have a ol/geom directory, so a geometrytype.js module fits nicely there.

On the other hand, we have a Property enum on ol.layer.Base (instead of a dedicated module for a ol.layer.LayerProperty enum) because

  1. ol.layer.LayerProperty was not used in many places, and
  2. we already have a Property enum on ol.layer.HeatMap, so this was sort of consistent with that.

After these changes, we can treat openlayers-internal/one-provide linter rule violations as errors. So it shouldn't be possible going forward to have multiple goog.provide() calls in a single file.

tschaub commented Dec 4, 2016 edited

The change in 294cb1f is required so the Compiler doesn't choke on our olx.js. We have types in there that may not be included in custom builds. This is a real problem for anybody creating custom builds (and it is not just about @typedef). We need to rework our build scripts so we always pass all source files to the Compiler. Without doing this, there will always be risks that types in olx.js are missing.

Update: see #6207 for a solution for the olx.js issue.

@tschaub tschaub added this to the v3.20.0 milestone Dec 6, 2016

Wow, what an effort. This looks great. Please merge.

tschaub commented Dec 6, 2016 edited

The change in 294cb1f should no longer be necessary after #6207. Prior to #6207, the make compile-examples target was failing because our (compilable) examples didn't explicitly require stuff that appears in olx.js. The change in 294cb1f was a specific fix to make our config/all-examples.json builds work, but #6207 should make it so custom builds work in general.

So, I've removed that commit. I'll merge if the build is green.

@tschaub tschaub merged commit 59e8027 into openlayers:master Dec 7, 2016

2 checks passed

continuous-integration/travis-ci/pr The Travis CI build passed
coverage/coveralls Coverage decreased (-0.005%) to 86.626%
@tschaub tschaub deleted the tschaub:no-extra-provides branch Dec 7, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment