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

Date() Nuance #26

Open
dathbe opened this issue Mar 26, 2024 · 1 comment
Open

Date() Nuance #26

dathbe opened this issue Mar 26, 2024 · 1 comment

Comments

@dathbe
Copy link

dathbe commented Mar 26, 2024

See this discussion for a nuance about the Date() recommendation offered by this repo:
BKeyport/MMM-Multimonth#21

It would be good if the recommendation engine was a little more context sensitive, but if that's too difficult (probably), then some additional guidance in the recommendation would be useful.

For example, it should probably say "Consider replacing" instead of "Replace".
And I read the post 3252 that is linked, and did not follow what the actual issue is, so a bit more description of when and why you'd make the change would be helpful.

Again, thanks for your work on this.

@KristjanESPERANTO
Copy link
Owner

Yes, this is a change whose point is perhaps not so clear without an explanation.

new Date() and new Date(Date.now()) are basically the same. The advantage of the second is that Date.now() can be overridden to test the behavior of the module at other dates/times. This can be very helpful for debugging time related issues.

There is a PR for the MagicMirror documentation which addresses this. When it's merged, we can link to that part. Until then maybe don't do this change in your PRs.

For example, it should probably say "Consider replacing" instead of "Replace".

Good point. I just changed it 🙂

Thanks for your work! I'm concentrating on other problems at the moment, so it's possible that things aren't progressing so quickly here. But don't let that hold you back from raising your questions and ideas.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants