Skip to content

Add snippet for plausible#695

Merged
jetstack-bot merged 1 commit intomasterfrom
plausible
Sep 9, 2021
Merged

Add snippet for plausible#695
jetstack-bot merged 1 commit intomasterfrom
plausible

Conversation

@james-w
Copy link
Contributor

@james-w james-w commented Sep 2, 2021

No description provided.

@jetstack-bot jetstack-bot added dco-signoff: no Indicates that at least one commit in this pull request is missing the DCO sign-off message. size/S Denotes a PR that changes 10-29 lines, ignoring generated files. labels Sep 2, 2021
@netlify
Copy link

netlify bot commented Sep 2, 2021 •
edited
Loading

✔️ Deploy Preview for cert-manager-website ready!

🔨 Explore the source changes: e8a62ef

🔍 Inspect the deploy log: https://app.netlify.com/sites/cert-manager-website/deploys/6138ec4d54c5da0007860dc8

😎 Browse the preview: https://deploy-preview-695--cert-manager-website.netlify.app

@maelvls
Copy link
Member

maelvls commented Sep 3, 2021 •
edited
Loading

Is this PR about swapping Google Analytics with Plausible.io pixel-based analytics?

It looks like we are still using Google Analytics:

website/config/_default/config.toml

Lines 89 to 92 in e38613d

[services.googleAnalytics]
id = "UA-78700849-2"
googleAnalytics = "UA-78700849-2"

website/layouts/partials/head.html

Line 20 in 45706c7

{{ template "_internal/google_analytics_async.html" . }}

Should we remove/disable the Google Analytics JS too?

Last note about the CNCF Sandbox: the website analytics need to be owned by the CNCF as per project-onboarding.md. With Plausible.io, would that be the case?

maelvls
maelvls reviewed Sep 3, 2021
View reviewed changes
layouts/partials/scripts.html Outdated
Comment on lines +16 to +24
<script type="text/javascript">
(function (d, u, h, s) {
h = d.getElementsByTagName('head')[0];
s = d.createElement('script');
s.async = 1;
s.src = u + new Date().getTime();
h.appendChild(s);
})(document, 'https://grow.clearbitjs.com/api/pixel.js?v=');
</script> No newline at end of file
Copy link
Member

@maelvls maelvls Sep 3, 2021

Choose a reason for hiding this comment

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

Can we have this added only in production builds like what is done for Google Analytics in layouts/partials/head.html?

E.g.:

{{ if eq (getenv "HUGO_ENV") "production" }}
{{ end }}

Copy link

@RiichardCC RiichardCC Sep 7, 2021

Choose a reason for hiding this comment

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

We need to keep GA in the header as well. pls do not remove this as the CNCF will wan it. The plausible snippet is just an initial view to see the analytics this service can produce until handover to the CNCF is complete.

Copy link
Contributor Author

@james-w james-w Sep 8, 2021

Choose a reason for hiding this comment

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

I have updated to put it in that file with the production check, thanks.

@james-w
Add snippet for plausible
e8a62ef 
Signed-off-by: James Westby <james@jameswestby.net>
@jetstack-bot jetstack-bot added dco-signoff: yes Indicates that all commits in the pull request have the valid DCO sign-off message. and removed dco-signoff: no Indicates that at least one commit in this pull request is missing the DCO sign-off message. labels Sep 8, 2021
@maelvls
Copy link
Member

maelvls commented Sep 8, 2021

Great!! Let's merge that.

/lgtm

@jetstack-bot jetstack-bot added the lgtm Indicates that a PR is ready to be merged. label Sep 8, 2021
maelvls
maelvls approved these changes Sep 9, 2021
View reviewed changes
Copy link
Member

@maelvls maelvls left a comment

Choose a reason for hiding this comment

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

Oops, apparently I also need to do a GitHub "Approve"

@jetstack-bot
Copy link
Contributor

jetstack-bot commented Sep 9, 2021

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: james-w, maelvls

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Details Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@jetstack-bot jetstack-bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Sep 9, 2021
@jetstack-bot jetstack-bot merged commit 357561b into master Sep 9, 2021
@jetstack-bot jetstack-bot mentioned this pull request Oct 19, 2021
Merged
@jetstack-bot jetstack-bot mentioned this pull request Oct 27, 2021
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@maelvls maelvls maelvls approved these changes

+1 more reviewer

@RiichardCC RiichardCC RiichardCC left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

@maelvls maelvls

Labels

approved Indicates a PR has been approved by an approver from all required OWNERS files. dco-signoff: yes Indicates that all commits in the pull request have the valid DCO sign-off message. lgtm Indicates that a PR is ready to be merged. size/S Denotes a PR that changes 10-29 lines, ignoring generated files.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@james-w @maelvls @jetstack-bot @RiichardCC