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

Adds Date and Time, update render. #2916

Merged
merged 13 commits into from
Mar 23, 2025
Merged

Conversation

theimpossibleleap
Copy link
Contributor

Hello! This was closed for being stale? Just want to get it merged with the updates. Thank you!

@tidbyt
Copy link

tidbyt bot commented Jan 11, 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 http.star requires review
Original Author The original author matches the PR author

@theimpossibleleap
Copy link
Contributor Author

theimpossibleleap commented Jan 31, 2025

Not sure what to write to ensure this doesn't stay stale and close again. Hopefully this gets approved and merged soon!

Thank you!

@theimpossibleleap
Copy link
Contributor Author

Hello @matslina - I see that PRs are continuing to close for being stale, even though they are waiting to merge. Is there a way to keep a PR from going stale? Any help is appreciated.

@theimpossibleleap
Copy link
Contributor Author

Hey @drudge - what does the Waiting on Author tag mean? It shows me that it's ready to merge it's just waiting for the review due to the http.star usage.

@theimpossibleleap
Copy link
Contributor Author

Hey @drudge - I updated branch. Can this be successfully merged? Thank you.

@theimpossibleleap
Copy link
Contributor Author

@drudge @matslina Hey y'all, can I get a status update on this? Any help is appreciated.

@danielsitnik
Copy link
Collaborator

@theimpossibleleap is there a reason why you're always converting the date/time to US/Pacific instead of the device's timezone?

You can get the device's timezone by reading the $tz configuration with the code below:

DEFAULT_TIMEZONE = "US/Pacific"

def main(config):
  device_tz = config.get("$tz", DEFAULT_TIMEZONE)

@theimpossibleleap
Copy link
Contributor Author

@theimpossibleleap is there a reason why you're always converting the date/time to US/Pacific instead of the device's timezone?

You can get the device's timezone by reading the $tz configuration with the code below:

DEFAULT_TIMEZONE = "US/Pacific"

def main(config):
  device_tz = config.get("$tz", DEFAULT_TIMEZONE)

Hey @danielsitnik!

No particular reason! Since it's a West Coast / Pacific NHL team and I, selfishly, built the app for myself, I just made it Pacific. I can definitely add $tz, I didn't know this existed for the Tidbyt. Thank you!

Adam

@danielsitnik
Copy link
Collaborator

danielsitnik commented Mar 21, 2025

@theimpossibleleap no worries, this "feature" was never documented anyway.
You had to learn it by looking at the other apps in the repo. 😁

If you could please make this small improvement, this would make the app usable by more users and more in line with the other apps that handle date/time this way. Then I'll gladly approve the PR!

@theimpossibleleap
Copy link
Contributor Author

Hey @danielsitnik - I made the suggested update! Let me know if it is correct. I appreciate you!

@drudge drudge merged commit aa473c9 into tidbyt:main Mar 23, 2025
2 checks passed
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

Successfully merging this pull request may close these issues.

3 participants