Get ad stats #2

Merged
merged 4 commits into from Jan 23, 2013

Conversation

Projects
None yet
2 participants
Member

nchrisdk commented Jan 23, 2013

this is a style thing... when we deal with namespaces and prefixes, I think it would make the code cleaner to extract all the namespaces and prefixes into separate file. This way we are consistent with the prefixes as well...

@nchrisdk nchrisdk commented on an outdated diff Jan 23, 2013

adform.getCampaigns = function(ticket, advertiser, callback) {
+ function formatCampaigns(rawCampaigns) {
@nchrisdk

nchrisdk Jan 23, 2013

Member

function not used

@nchrisdk nchrisdk commented on the diff Jan 23, 2013

adform.js
}, function(err, etree) {
- var campaigns = etree.findall('./s:Body/Campaigns/Campaign').map(function(campaign) {
+ var rawCampaigns = etree.findall('./s:Body/Campaigns/Campaign');
+ var campaigns = rawCampaigns.map(function(campaign) {
@nchrisdk

nchrisdk Jan 23, 2013

Member

I suspect that the partial function created above was meant to replace this callback...

@kesla

kesla Jan 23, 2013

Contributor

true, but skip that for now...

@nchrisdk nchrisdk commented on the diff Jan 23, 2013

adform.js
});
callback(null, campaigns);
});
-}
+};
+
+adform.getAdStats = function(ticket, campaign, startDate, endDate, callback) {
@nchrisdk

nchrisdk Jan 23, 2013

Member

a comment or two about how the startDate and endDate values should be formated: 2010-07-23

@kesla

kesla Jan 23, 2013

Contributor

nope, startDate and endDate is Date:s. See further down, they are then being transformed to that format.

Contributor

kesla commented Jan 23, 2013

@nchrisdk For the style thing (separating out namespaces) - I agree. But let's do that as a separate task further on. Do you mind create an issue for that?

kesla merged commit 5e7de90 into master Jan 23, 2013

kesla deleted the getAdStats branch Jan 23, 2013

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment