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

Jump to bottom

In /core/date.php, definition of method public static function factory($dateString, $timezone = null) is conflict with convention in reality and cause confusion #18641

Open
weixuezhang opened this issue Jan 18, 2022 · 1 comment
Open

In /core/date.php, definition of method public static function factory($dateString, $timezone = null) is conflict with convention in reality and cause confusion #18641

weixuezhang opened this issue Jan 18, 2022 · 1 comment
Labels
c: Documentation For issues related to in-app product help messages, or to the Matomo knowledge base. Easier debugging For issues that make troubleshooting issues for developers easier.
Milestone
For Prioritization

Comments

@weixuezhang
Copy link

In reality we are accustomed to local time + local timezone. The factory method with UTC time + local timezone is really confusing. If it aims at creating a local time, the class should be changed to another name.

Expected Behavior

Current Behavior

Possible Solution

Steps to Reproduce (for Bugs)

Context

Your Environment

  • Matomo Version:
  • PHP Version:
  • Server Operating System:
  • Additionally installed plugins:
  • Browser:
  • Operating System:
@weixuezhang weixuezhang added the Potential Bug Something that might be a bug, but needs validation and confirmation it can be reproduced. label Jan 18, 2022
@bx80
Copy link
Contributor

bx80 commented Jan 19, 2022

Hi @weixuezhang, thanks for raising this issue. I can understand why the Date::factory() parameters could cause confusion. This method is used extensively throughout the codebase and in plugins so it wouldn't be practical to rename it, but we could look at improving the method parameter names and the PHPDoc header to make it much clearer that the supplied date string should be in UTC.

@bx80 bx80 added c: Documentation For issues related to in-app product help messages, or to the Matomo knowledge base. Easier debugging For issues that make troubleshooting issues for developers easier. and removed Potential Bug Something that might be a bug, but needs validation and confirmation it can be reproduced. labels Jan 19, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
c: Documentation For issues related to in-app product help messages, or to the Matomo knowledge base. Easier debugging For issues that make troubleshooting issues for developers easier.
Projects
None yet
Milestone
For Prioritization
Development

No branches or pull requests
3 participants
@weixuezhang @bx80 @innocraft-automation