Skip to content

Conversation

@Rob-Keys
Copy link
Contributor

Small change but important, the app now gets the timezone from the config to adjust the time shown.

@tidbyt
Copy link

tidbyt bot commented Apr 23, 2025

⚠️ The automated review process is experimental and likely has bugs. Please bear with us as we iron out the kinks and enable you to ship changes at high velocity 🚀

Next Steps

Hello! Thank you so much for your change 🤜 🤛 . There are a few things you need to do:

  • Sign the CLA if you haven't already
  • Ensure your build is green! Any problem will display a proposed solution to try out
  • Get a review, either by Tidbyt Bot or by a Tidbyt engineer

Manual Review Required

Hang tight! A Tidbyt engineer will be by shortly to review your change. Here is what they will be looking for:

Test Details
App Dir All files are in a single app directory
🟡 Modules Usage of animation.star requires review
Original Author The original author matches the PR author

@danielsitnik
Copy link
Collaborator

@Rob-Keys I'm not sure this is going to work.

Usually we read the current timezone from the special config $tz, and the default value can also be passed to the function as the second argument. So the code should actually be:

timezone = config.get("$tz", "America/New_York")

@Rob-Keys
Copy link
Contributor Author

@danielsitnik Thank you, I adjusted the line to your suggestion.
The code I used came from the tidbyt dev website, https://tidbyt.dev/docs/build/clock-app, if it's possible to have that updated to match the proper way of fetching the timezone.

@danielsitnik
Copy link
Collaborator

Yes, sadly it has not been documented. :(
There's a PR (tidbyt/tidbyt.dev#620) open to fix it since last year but it hasn't been merged by the Tidbyt team.

@danielsitnik danielsitnik merged commit 0ba3307 into tidbyt:main Apr 24, 2025
2 checks passed
@tidbyt tidbyt locked as resolved and limited conversation to collaborators Apr 27, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants