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

amp-ad type weborama-display #2833

Merged
merged 4 commits into from
Apr 12, 2016
Merged

Conversation

bob-v-t
Copy link
Contributor

@bob-v-t bob-v-t commented Apr 7, 2016

Hi guys,

I hereby submit the pull request mentioned in issue number #2832. (backup link)

Please let me know if there is anything I can do to smoothen the integration process!

Thanks,
Bob

@googlebot
Copy link

Thanks for your pull request. It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

📝 Please visit https://cla.developers.google.com/ to sign.

Once you've signed, please reply here (e.g. I signed it!) and we'll verify. Thanks.


  • If you've already signed a CLA, it's possible we don't have your GitHub username or you're using a different email address. Check your existing CLA data and verify that your email is set on your git commits.
  • If you signed the CLA as a corporation, please let us know the company's name.

@bob-v-t
Copy link
Contributor Author

bob-v-t commented Apr 7, 2016

I signed it!

I've signed as an employee of the company Weborama.

PS: I used the same mail address that I used for my commits, but this is a different address from the one that I use for this Github account.

PS2: The travis CI build already failed before my additions.

@@ -117,6 +121,9 @@ export const adPreconnect = {
webediads: [
'https://eu1.wbdds.com',
],
'weborama-display': [
'https://cstatic.weborama.fr',
Copy link
Member

Choose a reason for hiding this comment

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

No need to repeat host names used in prefetch

@cramforce
Copy link
Member

Looks good. 2 small comments.

@cramforce
Copy link
Member

For the CLA: Did you submit a corporate or individual CLA?

@bob-v-t
Copy link
Contributor Author

bob-v-t commented Apr 11, 2016

About the CLA: It was a coorporate request.

@cramforce
Copy link
Member

@bob-v-t Great. The corporate CLAs can take a few business days to process. Personal is instant.

@bob-v-t
Copy link
Contributor Author

bob-v-t commented Apr 12, 2016

@cramforce I rebased and added the email and phone of our support this morning. I also just signed the CLA again, since the previous one was rejected. Let's hope this one works out 👍

@cramforce cramforce merged commit 570a0ae into ampproject:master Apr 12, 2016
@bob-v-t
Copy link
Contributor Author

bob-v-t commented Apr 13, 2016

Thanks @cramforce!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants