Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

replace 'opening_hours' with 'simple-opening-hours' #347

Open
mojoaxel opened this issue Oct 15, 2020 · 9 comments
Open

replace 'opening_hours' with 'simple-opening-hours' #347

mojoaxel opened this issue Oct 15, 2020 · 9 comments
Labels

Comments

@mojoaxel
Copy link
Member

mojoaxel commented Oct 15, 2020

opening_hours is very complex and has dependencies on moment.js, i18next-client and suncalc.
At the moment opening_hours accounts for about 37% of the complete bundle size!

It looks like simple-opening-hours also does, what we want but with a much smaller footprint!

@Kishore-abhimanyu
Copy link

can you assign this?

@mojoaxel
Copy link
Member Author

@Kishore-abhimanyu If you want to work on this, just do so and provide a pull-request 👍

@Kishore-abhimanyu
Copy link

Kishore-abhimanyu commented Oct 22, 2020

Raised a pull request. Please review and merge it. See #349.

@johnjohndoe
Copy link
Member

✔️ moment.js has been replaced with day.js in #343
Is replacing opening_hours.js still a topic?

@mojoaxel
Copy link
Member Author

Is replacing opening_hours.js still a topic?

Yes. This still would simplify things but also remove some functionality.

@johnjohndoe
Copy link
Member

... but also remove some functionality

Ooh. Like what? Do the libraries not provide the same functionality?

@mojoaxel
Copy link
Member Author

mojoaxel commented Nov 23, 2021

simple-opening-hours "only supports the human readable parts". e.g.

Mo-Sa 06:00-22:00
Mo-Fr 08:00-18:00; Sa 10:00-14:00
Mo-Fr 08:00-18:00; Sa,Su 10:00-20:00
Mo-Fr 08:00-12:00, We 14:00-18:00
Mo-Fr 08:00-12:00, 14:00-18:00
Mo-Fr 08:00-18:00; We off
24/7

opening_hours supports a much more compley syntax. I think we don't need this over-engineered syntax and should stick to the "human readable" stuff :-)

@johnjohndoe
Copy link
Member

Sounds reasonable at first hand. It would be interesting to find out how many of the current data points cannot be handled by days.js. The result would help to support the decision whether a migration make sense.

Related: Yesterday, I reached out to the Berlin Senat to sort out the "sonstige" data points which cannot be parsed by opening_hours.js. Pending response.

@johnjohndoe
Copy link
Member

Related: Yesterday, I reached out to the Berlin Senat to sort out the "sonstige" data points which cannot be parsed by opening_hours.js. Pending response.

Meanwhile, the number of "sonstige" markets has been reduced for Berlin. Still some to be sorted out.

@johnjohndoe johnjohndoe added the Abandoned Stale issues or pull requests label Jan 13, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants