Modify

Opened 8 months ago

Closed 8 months ago

#18577 closed enhancement (fixed)

[Patch] Update opening_hours.js

Reported by: simon04 Owned by: simon04
Priority: normal Milestone: 20.02
Component: Core validator Version:
Keywords: opening_hours Cc: ypid23

Description (last modified by simon04)

opening_hours.js has gotten many improvements since its last release on 2017-02-18: https://github.com/opening-hours/opening_hours.js/compare/v3.5.0...master
In particular a rather heavy external library could be removed via https://github.com/opening-hours/opening_hours.js/pull/250
When also excluding the country holiday data via https://github.com/opening-hours/opening_hours.js/pull/314, one can build a opening_hours+deps.min.js bundle with only 170KB whereas data/validator/opening_hours.js now has 316KB.

There are the few necessary modifications within JOSM: attachment:18577.patch

Two JOSM tests fail with the new version, however:

java.lang.AssertionError: 
Expected :isEmpty()
Actual   :<[service_times - Mo-Fr 00:00-00:30,04:00-00:30; Sa,Su,PH 00:00-24:00 <--- (This rule overwrites parts of the previous rule. This happens because normal rules apply to the whole day and overwrite any definition made by previous rules. You can make this rule an additional rule by using a "," instead of the normal ";" to separate the rules. Note that the overwriting can also be desirable in which case you can ignore this warning.) => Mo-Fr 00:00-00:30,04:00-00:30; Sa,Su,PH 00:00-24:00]>

	at org.hamcrest.MatcherAssert.assertThat(MatcherAssert.java:20)
	at org.openstreetmap.josm.data.validation.tests.OpeningHourTestTest.testCheckServiceTimeSyntax1(OpeningHourTestTest.java:190)
	at org.openstreetmap.josm.testutils.JOSMTestRules$CreateJosmEnvironment.evaluate(JOSMTestRules.java:670)
java.lang.AssertionError: opening_hours=sunset-sunrise open; sunrise-sunset closed is valid

Expected :isEmpty()
Actual   :<[opening_hours - sunset-sunrise open; sunrise-sunset closed <--- (This rule overwrites parts of the previous rule. This happens because normal rules apply to the whole day and overwrite any definition made by previous rules. You can make this rule an additional rule by using a "," instead of the normal ";" to separate the rules. Note that the overwriting can also be desirable in which case you can ignore this warning.) => sunset-sunrise open; sunrise-sunset closed]>

	at org.hamcrest.MatcherAssert.assertThat(MatcherAssert.java:20)
	at org.openstreetmap.josm.data.validation.tests.OpeningHourTestTest.testPresetValues(OpeningHourTestTest.java:235)
	at org.openstreetmap.josm.testutils.JOSMTestRules$CreateJosmEnvironment.evaluate(JOSMTestRules.java:670)

Attachments (3)

18577.patch (2.9 KB) - added by simon04 8 months ago.
opening_hours+deps.min.js (170.2 KB) - added by simon04 8 months ago.
18577-v2.patch (3.4 KB) - added by simon04 8 months ago.

Download all attachments as: .zip

Change History (9)

Changed 8 months ago by simon04

Attachment: 18577.patch added

comment:1 Changed 8 months ago by simon04

Description: modified (diff)
Summary: Update opening_hours.js[Patch] Update opening_hours.js

comment:2 Changed 8 months ago by simon04

Keywords: opening_hours added

Changed 8 months ago by simon04

Attachment: opening_hours+deps.min.js added

Changed 8 months ago by simon04

Attachment: 18577-v2.patch added

comment:3 Changed 8 months ago by simon04

The two above "false-positives" (in the JOSM sense) are solved via https://github.com/opening-hours/opening_hours.js/pull/317 and attachment:18577-v2.patch

@team, ypid23: Do we have to wait for a new release of opening_hours.js or can we apply the update rightaway?

comment:4 Changed 8 months ago by ypid23

I don’t feel comfortable making release now in that I am not up-to-date with the JavaScript world. The master branch is tested quite a bit so you can go with that. I don’t see an issue.

Thanks again simone04 for all your improvements!

comment:5 Changed 8 months ago by simon04

Milestone: 20.02

ypid23, thank you! Let's schedule this patch/update for the 20.02 JOSM release.

comment:6 Changed 8 months ago by simon04

Resolution: fixed
Status: assignedclosed

In 15815/josm:

fix #18577 - Update opening_hours.js

Modify Ticket

Change Properties
Set your email in Preferences
Action
as closed The owner will remain simon04.
as The resolution will be set.
The resolution will be deleted.

Add Comment


E-mail address and name can be saved in the Preferences.

 
Note: See TracTickets for help on using tickets.