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

New amp-ad extension needs a .md doc file #3271

Closed
tdrl opened this issue May 19, 2016 · 7 comments
Closed

New amp-ad extension needs a .md doc file #3271

tdrl opened this issue May 19, 2016 · 7 comments

Comments

@tdrl
Copy link

tdrl commented May 19, 2016

In the refactor from a builtin to an extension, amp-ad docs got left behind. I can see rationale for that, since the lazy script loading allows publishers to treat it as a builtin. But there should be at least some docs in extensions/amp-ad.

@zhouyx ?

@zhouyx
Copy link
Contributor

zhouyx commented May 23, 2016

Working on that. But basically no feature changed and the amp-ad docs in builtin already got updated. It seems to me adding docs to extensions/amp-ad would simply be adding a duplicate docs file there?

@rudygalfi
Copy link
Contributor

This is about updating https://github.com/ampproject/amphtml/blob/master/extensions/amp-sticky-ad/amp-sticky-ad.md, right?

I think we should have some content here. It should have the table that other extensions have with listings for Description, Availability, Required Script, Supported Layouts, and Examples. However, where it makes sense, it can heavily reference and make use of the documentation in amp-ad. For example, there should be a Behavior section defining how the sticky ad behaves and then it should hand off to amp-ad documentation to cover other topics.

@zhouyx
Copy link
Contributor

zhouyx commented May 23, 2016

I believe this is about adding new amp-ad.md because it's moved to extension .
For the amp-sticky-ad.md, The pull request is #3280. Thanks for the comment I will add the reference to amp-ad there.

@zhouyx
Copy link
Contributor

zhouyx commented May 24, 2016

@erwinmombay
So we want to redirect amp-ad docs from builtins to extensions?

@erwinmombay
Copy link
Member

yes we have to do it for www.ampproject.org/docs/* since we can do redirects there

@rudygalfi rudygalfi added this to the Current milestone May 25, 2016
@rudygalfi
Copy link
Contributor

Putting into Current milestone assuming this is getting actively worked on...

@zhouyx
Copy link
Contributor

zhouyx commented Oct 4, 2016

Thanks to @lannka, this should be closed by #4761 and #5026

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

No branches or pull requests

4 participants