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 video recording to DEI Speaker Series event page #784

Closed
wants to merge 1 commit into from

Conversation

nicole-brewer
Copy link
Contributor

@nicole-brewer nicole-brewer commented Apr 7, 2022

  • 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

@vsoch
Copy link
Member

vsoch commented Apr 7, 2022

Taking a look!

Sorry for delay was giving a talk!

@vsoch
Copy link
Member

vsoch commented Apr 7, 2022

That's weird no CircleCI preview - @nicole-brewer do you have Circle running on your account? I'll need to pull to preview.

@vsoch
Copy link
Member

vsoch commented Apr 7, 2022

Did you preview this and it worked? I think we need a different strategy for youtube embeds - this looks like it won't work.

image

What about just copying the share link from the video? If the include is broken (deemed malicious now) we should replace wherever it's used with the actual embed.

@vsoch
Copy link
Member

vsoch commented Apr 7, 2022

Also since you are PR-ing from main and this is going to be more than one commit, your main branch won't be synced with the main here (since we will squash and merge) and you are going to get a bunch of weird empty commits for future PRs. You might want to re-create your fork, pull, and then always check out a new branch when you PR.

@vsoch
Copy link
Member

vsoch commented Apr 7, 2022

Actually I take it back - it's the link! The previous one works still:
image

@@ -12,6 +12,7 @@ time:
end: 2022-03-11T20:00:00Z
---

{% include youtube-embed.html url="https://youtu.be/BdDTC4JsPvw" title="DEI Webinar - Allies, Collaboration, & Communication" %}
Copy link
Member

Choose a reason for hiding this comment

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

Try this one!

Suggested change
{% include youtube-embed.html url="https://youtu.be/BdDTC4JsPvw" title="DEI Webinar - Allies, Collaboration, & Communication" %}
{% include youtube-embed.html url="https://www.youtube.com/embed/BdDTC4JsPvw" title="DEI Webinar - Allies, Collaboration, & Communication" %}

@vsoch vsoch mentioned this pull request Apr 7, 2022
@vsoch vsoch closed this in #786 Apr 7, 2022
@vsoch
Copy link
Member

vsoch commented Apr 7, 2022

Merged in #786 - @nicole-brewer I didn't squash so since it's just one commit here your main branch should be okay :) Remember to check out a new branch next time! Thanks for adding the video!

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

2 participants