Remove Google Analytics Tracking Code

[erino]

As per #100

[calumgunn]

👍

[erino]

Maybe we should depreciate this method first?

[bensymonds]

deprecated :-)

[bensymonds]

As discussed, we can just delete this guy instead of deprecating.

[erino]

Should I remove the add_tracking_code helper too? https://github.com/withassociates/slices/blob/remove-google_analytics_tracking_code/app/helpers/pages_helper.rb#L83 too

[bensymonds]

Hmm good question. I think the decision to include admin activity or not is up to the Slices user. So I say remove it. There's nothing subtle here - Slices adds a Devise admin, so it's obvious how you would exclude it.

[bensymonds]

Seems ok to me 👍

[bensymonds]

Next release from master is v3 so we're ok break things, right? So nothing stopping us from merging this apart from two things which we've discussed:

• add entry to CHANGELOG.md
• see if any guides need updating

Both can happen in this PR.

[calumgunn]

Am I CHANGELOGging correctly?

[bensymonds]

@calumgunn I think that's what it would be if we are going to follow the existing style. But I propose we change the style. Two immediate things I think we should do:

1. Differentiate between breaking changes and other changes (so you can easily see what you might have to change in your app)
2. Link to the PR that made the change, so you can see more info about the change (cf. https://github.com/bbatsov/rubocop/blob/master/CHANGELOG.md)

Those are just off the top of my head. Anyone else got any ideas? @withassociates/with-core

[calumgunn]

@bensymonds I am totally in. Will make some changes now and see how we go.

[calumgunn]

@withassociates/with-core Check the new and cool changelog. Thoughts?

[erino]

👍

[bensymonds]

I like it. The only other thing I'm wondering is whether we should put a little bit more info in for the breaking changes, e.g. a few instructions to them on what they could/should do (so in this case it would be something along the lines of "include the js yourself"). In a lot of cases they can glean it from the linked PR, but it's usually pretty annoying to have to read through the whole PR to find it. If we put it in the changelog then it automatically becomes the upgrade guide for that version. So could rearrange slightly to this:

### Breaking changes
* [#101](https://github.com/withassociates/slices/pull/101): Remove `PagesHelper#google_analytics_tracking_code` and `PagesHelper#add_tracking_code`
  * You should include the GA javascript yourself.

[bensymonds]

(That's formatted crappily.)

[calumgunn]

@bensymonds Good plan. I've updated!

nb - To have nested lists in Markdown (Github Flavour), you indent by four spaces.

[bensymonds]

Ah ok, it says here that it's two spaces! I was too lazy to try four. Changelog looks 👍 to me. Did you check to see if any guides need updating? I'm thinking that those two things (changelog and guides) are updated (or confirmed they don't need updating) for each PR.

[calumgunn]

@bensymonds Couldn't see anything in the Guides for it, so we're good to go on this one!

[bensymonds]

Cool.