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

Add NSLondon February and March 2016 #108

Merged
merged 3 commits into from
May 17, 2016

Conversation

douglashill
Copy link
Contributor

No description provided.

@chriseidhof
Copy link
Owner

There seems to be some kind of CI issue, but I don't think it's your code... I think we'll need to fix that first.

On 13-04, Douglas Hill wrote:

You can view, comment on, or merge this pull request online at:

#108

-- Commit Summary --

  • Add NSLondon February and March 2016

-- File Changes --

M data/editions.yml (8)
M data/speakers.yml (2)
M data/videos.yml (27)

-- Patch Links --

https://github.com/chriseidhof/pomotv/pull/108.patch
https://github.com/chriseidhof/pomotv/pull/108.diff


You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
#108

NSLondon February 2016:
- language: English
speakers: [Raimon Lapuente, Gergely Orosz]
title: "Sorry Objc: It’s not you, it’s me"
Copy link
Contributor Author

Choose a reason for hiding this comment

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

https://travis-ci.org/chriseidhof/pomotv/builds/122938451#L2552

The build fails because the path generated for this video is build/events/nslondon/february-2016/gergely-orosz-raimon-lapuente-sorry-objc-it?s-not-you-it?s-me.html. The curly closing quotation marks, , are being turned into ?, which not allowed in URL paths.

I don’t know why the name is being processed like this. Compare these two, which have : and , and are wrapped in ":

title: "Sorry Objc: It’s not you, it’s me"
title: "Let’s Play: Refactor the Mega-Controller"

The second is working: it is processed into build/events/nsspain/2015/andy-matuschak-let’s-play-refactor-the-mega-controller.html

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I can recreate the failure locally with bundle exec middleman build --clean. I didn’t find this when I initially made the PR because I just used bundle exec middleman — and in that case I don’t think you see the problem unless you try to view a single video page, and I think the only way to do this is via search.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@chriseidhof Any ideas on this? While it would be nice to identify and fix the problem, if you think fixing is out-of-scope I can change the name to use straight quotation marks ' instead of proper ones .

Copy link
Owner

Choose a reason for hiding this comment

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

Sorry for the late reply @douglashill !

I think we should probably strip those characters when we generate the page names...

The code for that is in source/lib/video_helpers.rb. I think it's line 24. Do you have time to update the PR for this?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I had a little look but I am not familiar with the Ruby/middleman debugging tools, or really any part of the stack. I made #112 to track this. I suppose there’s no pressing need to fix it now. I’m not going to look into this further at the moment.

For now, I used straight quotes for the title causing the problem and rebased onto master, so we can get these videos up. 😄

@douglashill
Copy link
Contributor Author

Rebased to resolve conflicts and added NSLondon April.

@chriseidhof chriseidhof merged commit 7407f2b into chriseidhof:master May 17, 2016
@chriseidhof
Copy link
Owner

Awesome, thanks @douglashill

@douglashill douglashill deleted the nslondon-march-2016 branch June 19, 2016 20:54
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