bench: use site timezone for broadcast, don't expect offset decimal #171
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This was done because the javascript was erroneously interpreting the UTC offset string as a decimal, which caused the time to be off by an 30 minutes. This change should fix that. The site's timezone can be used to keep things consistent and avoid error from user input.
|
In future, would recommend using a rebase on to master if you need to get up to date as opposed to a merge. |
saxon-milton
approved these changes
Jun 26, 2024
|
Changes look good, but I wonder whether it even makes sense for the timezone to be changeable? Should we just use the site timezone and make it clear to the user that all times are based on the site timezone? I can't imagine many cases where a user would want to schedule a broadcast in a different location based on the timezone of their current location (mainly due to daylight hours etc... At very least maybe this should also be an advanced feature that gets hidden behind the advanced checkbox? |
ao-david
approved these changes
Jun 26, 2024
|
I think we should always use the site timezone. Otherwise we're simply
asking for trouble.
…On Thu, 27 June 2024, 8:59 am David (AusOcean), ***@***.***> wrote:
Changes look good, but I wonder whether it even makes sense for the
timezone to be changeable? Should we just use the site timezone and make it
clear to the user that all times are based on the site timezone? I can't
imagine many cases where a user would want to schedule a broadcast in a
different location based on the timezone of their current location (mainly
due to daylight hours etc...
At very least maybe this should also be an advanced feature that gets
hidden behind the advanced checkbox?
—
Reply to this email directly, view it on GitHub
<#171 (comment)>, or
unsubscribe
<https://github.com/notifications/unsubscribe-auth/AE3LX6GG2AC2MPBSLL23HDDZJNFGLAVCNFSM6AAAAABJ6AJPVWVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDCOJSG44DKMJXGE>
.
You are receiving this because your review was requested.Message ID:
***@***.***>
|
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This was done because the javascript was being used that interpreted the UTC offset string as a decimal, which caused the time to be off by an 30 minutes. This change should fix that.
The site's timezone can be used to keep things consistent and avoid error from user input.