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

📖 Format analytics vendors list and update links #33140

Merged
merged 9 commits into from
Mar 10, 2021

Conversation

alanorozco
Copy link
Member

Comment on lines +48 to +52
[tip type="important"]

Do not use both the `adobeanalytics` and `adobeanalytics_nativeConfig` types on the same page. If you attempt to do so, you can generate errors in the browser console and double-count visitors.

[/tip]
Copy link
Member Author

@alanorozco alanorozco Mar 8, 2021

Choose a reason for hiding this comment

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

I didn't know this document lived on amp.dev, so previously I used warning syntax that would look fine on Github. It doesn't look right on amp.dev so might as well use a tip:

image

@@ -395,7 +399,7 @@ Adds support for Nielsen Marketing Cloud. More details can be found at [Nielsen

Type attribute value: `oewa`

Adds support for `[OEWA](https://www.oewa.at)`. Requires a copy of [amp-analytics-oewa.html](http://www.oewa.at/fileadmin/downloads/amp-analytics-oewa.html) on a different subdomain than the including AMP file ([why?](https://github.com/ampproject/amphtml/blob/master/spec/amp-iframe-origin-policy.md)). The file must be served via HTTPS. For example, if your AMP files are hosted on `www.example.com`, then `amp-analytics-oewa.html` needs to be on another subdomain such as `oewa-amp.example.com`. More details for adding OEWA support can be found [here](http://www.oewa.at/Implementierung).
Adds support for [OEWA](https://www.oewa.at). Requires a copy of [amp-analytics-oewa.html](http://www.oewa.at/fileadmin/downloads/amp-analytics-oewa.html) on a different subdomain than the including AMP file ([why?](https://github.com/ampproject/amphtml/blob/master/spec/amp-iframe-origin-policy.md)). The file must be served via HTTPS. For example, if your AMP files are hosted on `www.example.com`, then `amp-analytics-oewa.html` needs to be on another subdomain such as `oewa-amp.example.com`. More details for adding OEWA support can be found [here](http://www.oewa.at/Implementierung).
Copy link
Member Author

Choose a reason for hiding this comment

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

Several URLs and links were formatted as code. Made clickable links.

image

> ⚠️ ~~`webtrekk`~~ is deprecated. **Use `webtrekk_v2` instead.**
[tip type="important"]

<span style="text-decoration: line-through;">`webtrekk`</span> is deprecated. **Use `webtrekk_v2` instead.**
Copy link
Member Author

Choose a reason for hiding this comment

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

amp.dev's markdown interpreter won't do strikethrough with ~~, use CSS.

image

@alanorozco alanorozco marked this pull request as ready for review March 8, 2021 18:57
### SimpleReach

Type attribute value: `simplereach`

Adds support for SimpleReach. Configuration details can be found at `http://docs.simplereach.com/dev-guide/implementation/google-amp-implementation`.
Adds support for SimpleReach. Configuration details can be found at [docs.simplereach.com](http://docs.simplereach.com/dev-guide/implementation/google-amp-implementation).
-->
Copy link
Member Author

Choose a reason for hiding this comment

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

Section is kept as a comment intentionally (see top of file).

Comment on lines +450 to +454
<!--
(The following link 404s, it's unclear whether vendor is still supported or if docs are now private.)

Adds support for Piano. Configuration details can be found at `http://vx.piano.io/javascript-tracking-amp`
-->
Copy link
Member Author

Choose a reason for hiding this comment

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

Thanks to check-links, this also became a link update.

@alanorozco alanorozco changed the title 📖 Format analytics vendors list 📖 Format analytics vendors list and update links Mar 8, 2021
alanorozco and others added 2 commits March 8, 2021 12:46
…o-patch-5

# Conflicts:
#	extensions/amp-analytics/analytics-vendors-list.md
@@ -455,7 +467,11 @@ Type attribute value: `piStats`

Type attribute value: `pressboard`

Adds support for Pressboard. Configuration details can be found at [help.pressboard.ca] `http://help.pressboard.ca/publisher-resources/getting-started/implementing-google-amp`.
<!--
(Link 404s, unclear whether vendor is still supported.)
Copy link
Member Author

Choose a reason for hiding this comment

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

@omar84

Hi! It looks like you originally added this analytics config on #18169

The original documentation link 404s, and it's unclear whether AMP is still supported by this vendor.

  • Is the vendor integration still active?
  • If so, is there a newer link?

Thanks.

Copy link
Contributor

Choose a reason for hiding this comment

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

Hi @alanorozco
I'm with Pressboard (the vendor) and taking over from @omar84. We're still actively supporting AMP. Any assistance on how I can get started would be greatly appreciated. I'll setup a new documentation link asap.

Copy link
Member Author

Choose a reason for hiding this comment

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

@tiamkorki Awesome, good to know. The implementation is still in place, not sure if it requires any later work. Please ping me or @micajuine-ho on Slack if you need specific assistance.

Copy link
Contributor

@micajuine-ho micajuine-ho left a comment

Choose a reason for hiding this comment

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

Approved. Will defer to you on how long we should wait for a response from @omar84.

@alanorozco alanorozco merged commit f7d0c34 into master Mar 10, 2021
@alanorozco
Copy link
Member Author

@micajuine-ho We can submit as-is, and update if vendor replies.

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.

None yet

4 participants