feat(validate_config): add warning about timezone#5731
feat(validate_config): add warning about timezone#5731stevenjoezhang wants to merge 1 commit intomasterfrom
Conversation
How to testgit clone -b tz https://github.com/hexojs/hexo.git
cd hexo
npm install
npm test |
Pull Request Test Coverage Report for Build 20736158720Details
💛 - Coveralls |
| const machineTimezone = moment.tz.guess(); | ||
| if (configTimezone.name !== machineTimezone) { |
There was a problem hiding this comment.
Do we have to guess and warn about a mismatch here? E.g., running Hexo on CI may have a different timezone, intentionally.
There was a problem hiding this comment.
Yes that the warning will always be displayed in CI environments. However, I think it should be displayed.
If the user's permalink setting includes time-based parts, it will also change the URLs of their posts.
As @stevenjoezhang mentioned in the above comment at #5720 (comment), my concern is that URLs may change.
Currently, URLs are always generated using the LocalTimeZone (machine's timezone) for date/time formatting. If we change it to use the Timezone from _config.yml for generating URLs in the next major version, and users are unaware that their machine's timezone and the timezone in _config.yml are inconsistent, the generated URLs could unintentionally change.
However, I do think it might be acceptable to address this by including a migration guide in the release notes instead of showing the warning.
There was a problem hiding this comment.
What do you think about only giving this warning if this is not in CI? (In CI, there is nothing users can do anyway; they need to validate this on his/her local machine).
Also, since this only affects permalink patterns with time, we may emit this warning only when one of the permalink patterns uses time.
There was a problem hiding this comment.
@yoshinorin @SukkaW I believe that displaying a warning message is necessary both locally and in CI environments, because there is a possibility that links may change. Showing a clear warning is more reasonable than remaining completely silent. For example, if a user does not configure the time zone correctly and this causes the article links generated by CI to change, the issue can still be discovered by checking the CI logs.
I think we still have two tasks to complete:
- Check whether the permalink patterns contain time-related parameters, and use that as a condition for displaying the warning message.
- Write a migration notice in the release notes to inform users how to properly configure the time zone.
There was a problem hiding this comment.
@SukkaW @stevenjoezhang
I agree with only showing the warning when the permalink is affected.
As for CI environments, I don't think there's a reliable way to detect them. Even if there were, it would add unnecessary implementation complexity, so I think always showing the warning is fine. Instead of suppressing the warning in CI environments, it could help to make the warning message clearer, for example by including a link to the relevant documentation or this discussion.
4 participants
What does it do?
See #5720
Screenshots
Pull request tasks