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

Convert group times to PyCon time, to match session times #31

Closed
wants to merge 4 commits into from

Conversation

hugovk
Copy link
Contributor

@hugovk hugovk commented Apr 6, 2023

Ned's keynote is at 9:30 PyCon time: https://us.pycon.org/2023/schedule/talks/

Before

I'm in Helsinki/EEST, 9 hours ahead of PyCon/Salt Lake City/MDT.

It shows SLC time for the keynote (9:30 – 10:10) but my local time for the group (Fri - 18:00):

image

Fix

Let's set the same timeZone: "MST7MDT" option when converting the group time, as we do for the session start and end times:

"timeStart": start.toLocaleTimeString([], {timeZone: "MST7MDT", hour: 'numeric', minute:'2-digit'}).toLowerCase(),
"timeEnd": end.toLocaleString([], {timeZone: "MST7MDT", hour: 'numeric', minute:'2-digit'}).toLowerCase(),

After

image

Also

Bump GitHub Actions, run CI for pushes as well so contributors can confirm it passes before opening PRs.

@@ -32,5 +32,6 @@ plugins/ios.json
$RECYCLE.BIN/

.DS_Store
capacitor.config.json
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why was this ignored?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Whoops, I missed during review! I think the correct behavior is to gitignore the generated one: https://github.com/psf/pycon-us-mobile/blob/main/android/app/src/main/assets/capacitor.config.json

See ionic-team/capacitor#5713

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

After installing/building/testing, capacitor.config.json showed up for me in the root.

Its contents are:

{}

@miketheman Should it be gitignored or something else?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@hugovk after reading some more about capacitor configs, we're using the typescript-based config, and the first build renders the empty file.

I think the right action for this codebase is to actually commit the empty one to denote "nothing to see here" and .gitignore the generated ones from the assets directory, so that any changes to the .ts file will update the generates assets, and not cause git churn.

Dunno how worthwhile it is though - I'll leave that up to you to decide.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, done in PR #38

@ewdurbin ewdurbin mentioned this pull request Apr 6, 2023
@ewdurbin
Copy link
Member

ewdurbin commented Apr 6, 2023

addressed as part of #33, please reopen with some of the other best practice changes 💜

@ewdurbin ewdurbin closed this Apr 6, 2023
@hugovk hugovk deleted the group-time-match-session-time branch April 7, 2023 13:02
@hugovk
Copy link
Contributor Author

hugovk commented Apr 7, 2023

Confirmed fix in the latest update, thank you! 🕤:tada:

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.

None yet

3 participants