Skip to content
This repository has been archived by the owner on Jul 21, 2019. It is now read-only.

updates CFP OPEN footer to use time comparisons #641

Merged
merged 3 commits into from
Aug 27, 2018

Conversation

tgross
Copy link
Contributor

@tgross tgross commented Aug 24, 2018

For #476 and #487 (but incomplete w/o data changes)

The existing footer takes the current time (build time), casts it to a string, and compares it with the CFP start/end times which are also being re-cast to strings here (twice!).

In order to allow us to use timezones as part of our comparison and not have the (lossy) formatting involved in comparisons, use the time template function and directly compare against now without typecasting.

ref https://gohugo.io/functions/time/
ref https://gohugo.io/functions/now/

I've demonstrated an example with singapore 2018 and I've tested this out by switching it to my own TZ and adjusting the end date and up and down and watching it change in the rendered example site. But we'll need to update data in the devopsdays-web repo to get the full effect. I'll make a separate PR for that (automating the fix to add proper TZs to events that haven't happened yet if I can).

cc @bridgetkromhout


This change is Reviewable

The existing footer takes the current time (build time), casts it to a
string, and compares it with the CFP start/end times which are also
being re-cast to strings here (twice!).

In order to allow us to use timezones as part of our comparison and not
have the formatting involved in comparisons, use the `time` template
function and directly compare against `now` without typecasting.

ref https://gohugo.io/functions/time/
ref https://gohugo.io/functions/now/
@bridgetkromhout
Copy link
Contributor

bridgetkromhout commented Aug 24, 2018

Ooh, exciting! Thank you, Tim! It does seem like the display gracefully degrades if they've got the old style set, which is probably good since often returning events don't run our scripts to instantiate their next event with the latest and greatest, but instead copy their previous event yaml, update dates, and remove sponsors/schedule from the previous year.

I do notice that in the view we can see in the preview (https://deploy-preview-641--devopsdays-theme.netlify.com/speaking) it ends up looking a little confusing with the long time string:

screen shot 2018-08-24 at 2 58 52 pm

I wonder if we should format the display in https://github.com/devopsdays/devopsdays-theme/blob/master/layouts/partials/speaking.html to make the time part of the date a little more human-readable on that page?

The call for participation for a devopsdays event helps each city’s local organizers find speakers for their event. Follow the talk proposal submission process for a given city if you’d like to submit an idea to their CFP! These are the cities which currently have an open call for participation.
GitHub
devopsdays-theme - Hugo theme for devopsdays.org

@tgross
Copy link
Contributor Author

tgross commented Aug 24, 2018

I wonder if we should format the display in https://github.com/devopsdays/devopsdays-theme/blob/master/layouts/partials/speaking.html to make the time part of the date a little more human-readable on that page?

Yup, I can do that no problem. Pretty much anywhere we display those values without formatting them first we'll see that issue. I'll try to take a look through to see if there's anywhere else.

GitHub
devopsdays-theme - Hugo theme for devopsdays.org

@bridgetkromhout bridgetkromhout changed the title updates CFP OPEN footer to use time comparions updates CFP OPEN footer to use time comparisons Aug 24, 2018
@bridgetkromhout
Copy link
Contributor

bridgetkromhout commented Aug 25, 2018

To clarify for data updates: the live site is populated by the yaml files in https://github.com/devopsdays/devopsdays-web/tree/master/data/events and (for recent and upcoming events) the corresponding files in https://github.com/devopsdays/devopsdays-web/tree/master/content/events, with static files in https://github.com/devopsdays/devopsdays-web/tree/master/static/events.

Note to provide more context for @tgross: archived events are out of scope for this particular issue, but in case you end up needing to know, they are handled in a couple of places for reasons of build time, cruft, legacy, the usual.

@mattstratton and I would like to keep past events in the same place as the live site. But realistically, the effect on build time is that processing sites which haven't changed adds too much extra time. This is something we'll probably revisit.

GitHub
devopsdays-web - This is the website for devopsdays.org
GitHub
devopsdays-web - This is the website for devopsdays.org
GitHub
devopsdays-web - This is the website for devopsdays.org
GitHub
devopsdays-web - This is the website for devopsdays.org
GitHub
devopsdays-legacy - Contains the static files for devopsdays.org prior to Hugo migration

@tgross
Copy link
Contributor Author

tgross commented Aug 25, 2018

I wonder if we should format the display in https://github.com/devopsdays/devopsdays-theme/blob/master/layouts/partials/speaking.html to make the time part of the date a little more human-readable on that page?

I just pushed a commit that fixes this by restoring the formatting to be exactly what the old formatting is (ex. "2018-08-25"). This doesn't include the time but we can get arbitrarily bikesheddy about how you'd like to display that if we want, or whether to display the time at all. 😀 Let me know if you'd like to have the time in displayed text and I can do that.

GitHub
devopsdays-theme - Hugo theme for devopsdays.org

@tgross
Copy link
Contributor Author

tgross commented Aug 25, 2018

To clarify for data updates: the live site is populated by the yaml files in

Yup, understood. I'm working up a PR for data changes specifically for the CFP dates, but I'll have another pair of PRs for the registration and start/end dates for #211.

Note to provide more context for @tgross: archived events are out of scope for this particular issue, but in case you end up needing to know, they are handled in a couple of places for reasons of build time, cruft, legacy, the usual

Makes sense. Build times are surprisingly long (for a hugo site) as it is, but there's also just a lot of stuff here for the build process to shuffle around!

@mattstratton
Copy link
Member

Yep. I’m really sure that once my craptastic code gets better optimized it will drastically improve!!

@tgross
Copy link
Contributor Author

tgross commented Aug 25, 2018

I was just re-testing this locally and while this solves #476 it doesn't solve #487 like I hoped. That appears to be entirely client-side where we have just the 2018-08-23 strings and nothing to do with our timestamp handling.

@bridgetkromhout
Copy link
Contributor

This doesn't include the time but we can get arbitrarily bikesheddy about how you'd like to display that if we want, or whether to display the time at all. 😀 Let me know if you'd like to have the time in displayed text and I can do that.

So what happens is people submitting talks get rules-lawyer-y about what even is "late" and are they actually late. This can be somewhat alleviated if a time is displayed. It's not crucial, but if we're going to store a time at all, events may get some benefit by displaying it. (Insert caveat about, as we discussed in the PR, the local start time being 00:00 and the local end time being 23:59 are probably our safest defaults).

@bridgetkromhout
Copy link
Contributor

I was just re-testing this locally and while this solves #476 it doesn't solve #487 like I hoped. That appears to be entirely client-side where we have just the 2018-08-23 strings and nothing to do with our timestamp handling.

Alas! But, it's still an improvement.

@bridgetkromhout
Copy link
Contributor

Please take a look at my proposed change to the reference doc, and please correct it if I have it wrong. (And perhaps make further edits that are warranted, to the other fields?)

@bridgetkromhout
Copy link
Contributor

I want to make sure this particular doc changes goes in along with this PR because I think this feature will be the most helpful if people understand how to set it. When @tgross thinks the docs are right, this can be merged.

@tgross
Copy link
Contributor Author

tgross commented Aug 27, 2018

Please take a look at my proposed change to the reference doc, and please correct it if I have it wrong. (And perhaps make further edits that are warranted, to the other fields?)

Yup, that LGTM. There's a utility script in the other repo that copies a template for the event data YAML. We should probably update this line there too https://github.com/devopsdays/devopsdays-web/blob/master/utilities/examples/data/events/yyyy-city.yml#L8. I'll add that change to devopsdays/devopsdays-web#5213

@bridgetkromhout
Copy link
Contributor

We should probably update this line there too

Cool. So, to clarify, is the cfp end date the only variable we should be listing in the full format (not just YYYY-MM-DD)? It's fine for it to be one more-specific choice - just want to make sure we're documenting this enhancement accurately.

@tgross
Copy link
Contributor Author

tgross commented Aug 27, 2018

My thinking is that it's always correct to have the data include dates in the full format w/ TZ (so long as we remember in the theme to use dateFormat in the templates), whereas it's not always correct to not include it. And that if you want to change the format in the templates later on having the time and TZ info will allow them render correctly without data changes.

@bridgetkromhout bridgetkromhout merged commit 3394395 into devopsdays:master Aug 27, 2018
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.

3 participants