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

Added December DEI speaker series event page #569

Merged
merged 6 commits into from
Oct 29, 2021

Conversation

nicole-brewer
Copy link
Contributor

Checklist:

  • I have posted the link for the PR in the usrse slack (#website) to ask for reviewers
  • I have previewed changes locally

cc @usrse-maintainers

@danielskatz
Copy link
Contributor

I'm confused by why there is just the URL check here, but not the action to build the site and create a preview. Did something change in our CI config?

@danielskatz
Copy link
Contributor

It also looks like the MBARI job (https://www.mbari.org/software_engineer/ "Software Engineer: Monterey Bay Aquarium Research Institute Posted: Sep 20, 2021") is not longer posted, so we probably need to remove that posting as part of this PR too

@nicole-brewer
Copy link
Contributor Author

No I just added a file in _event/ and added a line to .gitignore. I'm also confused by the message. It's not a url relating to my post.

@danielskatz
Copy link
Contributor

the way our CI works is that every change does a check of all URLs to see if anything has broken, and if so, then the PR needs to be changed to also fix the broken URL.

@nicole-brewer
Copy link
Contributor Author

Yes, I'll delete it

@danielskatz
Copy link
Contributor

I think you should remove lines 37-41 of _data/jobs.yml to fix this

@danielskatz
Copy link
Contributor

It's not your fault, you just happened to make the first PR after the URL broke and get stuck fixing it

@danielskatz
Copy link
Contributor

Alternatively, perhaps you could change the expiration date for the job to yesterday

@nicole-brewer
Copy link
Contributor Author

Right, I see. Thanks 😊

@vsoch
Copy link
Member

vsoch commented Oct 29, 2021

@danielskatz the urlchecker is agnostic to the expires date - it will still trigger that the URL is 404.

@danielskatz
Copy link
Contributor

so what's the right fix? remove the job?

@vsoch
Copy link
Member

vsoch commented Oct 29, 2021

Your first suggestion was correct - yes - the job should be removed! Normally these get detected on a nightly run and I take care of it, but @nicole-brewer you are unlucky that the job 404'd between last night's run and now.

@danielskatz
Copy link
Contributor

Also, @vsoch - do you know why there was no site build/preview generated?
I see that now this is happening, but don't understand why it didn't before

@vsoch
Copy link
Member

vsoch commented Oct 29, 2021

@danielskatz I'm not 100% but I would suspect it's just a bloop in the checks API sending back the check from Circle. It actually did run (this says 22/23 minutes ago https://app.circleci.com/pipelines/github/nicole-brewer/usrse.github.io/7/workflows/cc0378f3-ff12-4733-9d3b-5413313d6d2b) but didn't show up here.

@danielskatz
Copy link
Contributor

To me, the two "Tech"s in the abstract should be "tech", but if this is the abstract the speaker provided, that's ok I guess

@danielskatz
Copy link
Contributor

I prefer the "Dr"s to be "Dr."s but again, not a big deal

@danielskatz
Copy link
Contributor

I wonder if we should change the data/time info to link to "find this in your own time zone" as well?

@nicole-brewer
Copy link
Contributor Author

How do I do that?

@vsoch
Copy link
Member

vsoch commented Oct 29, 2021

@danielskatz the page should already render the event in the viewer's browser's time zone. Let me double check this one.

@vsoch
Copy link
Member

vsoch commented Oct 29, 2021

yeah see I'm in Mountain and I see Mountain! ⭐

image

@danielskatz
Copy link
Contributor

@vsoch
Copy link
Member

vsoch commented Oct 29, 2021

Add to calendar is also correct zone

image

@danielskatz
Copy link
Contributor

since I do see the right time on the top, but I also see this
Screen Shot 2021-10-29 at 16 34 06
which just shows ET and PT

@vsoch
Copy link
Member

vsoch commented Oct 29, 2021

yeah that's hard coded text in the Markdown: https://github.com/USRSE/usrse.github.io/pull/569/files#diff-3a753bbb9576a9bd8401a6c332cc987b127b0e5657773e183226a39e51fb0979R21. I guess I think it's okay generally because it gets the span of the US? Our other events surely vary in how they designate this part - some just have Eastern time. I think probably if there is to be a standard it should be discussed in the chat and then implemented across events.

@danielskatz
Copy link
Contributor

that's reasonable - just pointing out that we could make this friendlier to other time zones too, but not a big deal either way

@lparsons
Copy link
Member

lparsons commented Oct 29, 2021

since I do see the right time on the top, but I also see this Screen Shot 2021-10-29 at 16 34 06 which just shows ET and PT

This follows the suggestions here: https://github.com/USRSE/usrse.github.io#3-how-do-i-add-an-event

You should preview your page to make sure that the time zone is rendering as you'd expect, and it's also helpful to write out a listing of timezones in the content, e.g.,:

The next community call will be on August 12, 2021 at 12ET/11CT/10MT/9PT.

This is helpful because people in the community come from many different time zones and also travel, and it's nice to quickly see the mapping for other time zones that are close by.

@nicole-brewer
Copy link
Contributor Author

Oh okay. I'll update one more time.

Copy link
Member

@lparsons lparsons left a comment

Choose a reason for hiding this comment

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

LGTM, a few minor suggestions I leave to your discretion:

@vsoch
Copy link
Member

vsoch commented Oct 29, 2021

lol I totally wrote something similar in the README

image

Must have been 3am Vanessa because 3pm Vanessa doesn't remember that 😆

@danielskatz
Copy link
Contributor

I think this is ready to go

@danielskatz danielskatz merged commit 0ad2051 into USRSE:main Oct 29, 2021
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

4 participants