Skip to content
This repository has been archived by the owner. It is now read-only.

Create new tracking codes module #1

Closed
ldusan84 opened this issue Mar 24, 2015 · 4 comments
Closed

Create new tracking codes module #1

ldusan84 opened this issue Mar 24, 2015 · 4 comments

Comments

@ldusan84
Copy link
Contributor

@ldusan84 ldusan84 commented Mar 24, 2015

No description provided.

@ldusan84

This comment has been minimized.

Copy link
Contributor Author

@ldusan84 ldusan84 commented Mar 24, 2015

Hi @shahbaztariq @tonegolf71 @louisnorthmore @chesuch @georgeschiopu

As we discussed on Friday meeting, here is my first version of the module with Criteo implementation. I would like to discuss a few things, just to make sure that we are going in the right direction.

Templates for Criteo are very similar, but different in some parts. For now there are 5 separate templates, for me it would make sense that we place the part that changes depending on a page in a child template and then call it when necessary, although this would make the layout more complicated.

Every template has it's block that inherits from abstract block. Some functionalities that are used across tracking code systems are placed in helpers.

Would you rather place the checks if the feature is enabled in layout or in the block?

Let me know if you think this is the correct approach and then we can continue with the other implementations.

Thanks
Dusan

@shahbaztariq

This comment has been minimized.

Copy link

@shahbaztariq shahbaztariq commented Mar 24, 2015

Hello @ldusan84 @tonegolf71 @louisnorthmore @chesuch @georgeschiopu you can see my pull request here: #2

@ldusan84

This comment has been minimized.

Copy link
Contributor Author

@ldusan84 ldusan84 commented Mar 25, 2015

@shahbaztariq Looks good to me so I'll merge.

@jakespace48

This comment has been minimized.

Copy link

@jakespace48 jakespace48 commented Apr 27, 2015

I will close this as the work was complete.

@jakespace48 jakespace48 removed their assignment Apr 27, 2015
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
3 participants
You can’t perform that action at this time.