Non-blocking findings from sonnet-reviewer on PR #15 (merged). Capture for cleanup before upstream PR review on TerriaJS#7835.
-
Dead code: if (!t) return undefined guard in TimeOverrideStratum.discreteTimes never fires (TerriaJS object-traits always materialize wrapper). Remove or replace with field-level guard for clarity.
-
Silent failure: time: { start: "X", stop: "Y" } without period produces no discreteTimes and no diagnostic. Add a console.warn so catalog authors see the misconfiguration: "WebMapTileServiceCatalogItem: time.start + time.stop given without time.period; discrete time list is empty. Add a period (e.g., 'P1D')."
-
Untested contract: defaultValue outside the values list sets currentTime to a string not in discreteTimes. DiscretelyTimeVaryingMixin behavior in that case is unspecified. Either: (a) constrain — set currentTime to the closest matching value, OR (b) document the contract explicitly + add a spec asserting the chosen behavior.
Estimated: 30 min total. Bundle into a small follow-up PR before pushing to upstream review on TerriaJS#7835.
Non-blocking findings from sonnet-reviewer on PR #15 (merged). Capture for cleanup before upstream PR review on TerriaJS#7835.
Dead code:
if (!t) return undefinedguard inTimeOverrideStratum.discreteTimesnever fires (TerriaJS object-traits always materialize wrapper). Remove or replace with field-level guard for clarity.Silent failure:
time: { start: "X", stop: "Y" }withoutperiodproduces nodiscreteTimesand no diagnostic. Add aconsole.warnso catalog authors see the misconfiguration: "WebMapTileServiceCatalogItem: time.start + time.stop given without time.period; discrete time list is empty. Add a period (e.g., 'P1D')."Untested contract:
defaultValueoutside thevalueslist setscurrentTimeto a string not indiscreteTimes.DiscretelyTimeVaryingMixinbehavior in that case is unspecified. Either: (a) constrain — set currentTime to the closest matching value, OR (b) document the contract explicitly + add a spec asserting the chosen behavior.Estimated: 30 min total. Bundle into a small follow-up PR before pushing to upstream review on TerriaJS#7835.