Skip to content
This repository has been archived by the owner on Nov 30, 2024. It is now read-only.

Add baidu in analytics provider, add tip for enabling analytics plugin #158

Closed
wants to merge 4 commits into from
Closed

Conversation

themez
Copy link

@themez themez commented May 12, 2013

analytics only works when safe is true, it's better to put a tip there.

@groundh0g groundh0g added this to the v 0.5.0 milestone Apr 4, 2015
@groundh0g groundh0g self-assigned this Apr 4, 2015
@groundh0g
Copy link
Collaborator

Related issue #84. site.safe may be obsoleted.

@@ -95,6 +95,7 @@ JB :
colorscheme: light

# Settings for analytics helper
# Set 'site.safe' to true to enable analytics plugin
Copy link
Collaborator

Choose a reason for hiding this comment

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

@themez we've moved away from using site.safe to decide if the site is in production mode. I'm killing this line and merging the rest.

Copy link
Collaborator

Choose a reason for hiding this comment

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

@themez, crap. I can't edit inline. I will have to resubmit this using my account, or you can make the tweak on your end and I'll merge. Lemme know.

Copy link
Collaborator

Choose a reason for hiding this comment

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

@themez, double crap. This now has merge conflicts. Will have to create new PR. If you don't have time or desire, I'll handle it. Thanks for contribution!

Copy link
Author

Choose a reason for hiding this comment

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

Hi ground0g, I've already deleted my fork repo, cannot make a new pr now.. It would be good if you can fix it with your account.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Will do. Thanks for the contribution! Sorry it's taken so long to review.

@groundh0g
Copy link
Collaborator

Isolated. New analytics provider. Low risk. BUT ... Cannot merge now. See line-item comments.

@groundh0g
Copy link
Collaborator

From @themez:

Hi ground0g, I've already deleted my fork repo, cannot make a new pr now.. It would be good if you can fix it with your account.

Will do. Thanks for the contribution! Sorry it's taken so long to review.

@groundh0g
Copy link
Collaborator

Made edits by hand in local repo. Pushing this weekend.

@groundh0g groundh0g closed this Oct 17, 2015
groundh0g added a commit to groundh0g/jekyll-bootstrap that referenced this pull request Oct 18, 2015
…an account to test, but this is isolated code and the risk to break existing analytics modules is low.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants