Skip to content
This repository has been archived by the owner on Dec 15, 2021. It is now read-only.

Implement Calculated Metrics API #205

Closed
randyzwitch opened this issue Feb 2, 2017 · 7 comments
Closed

Implement Calculated Metrics API #205

randyzwitch opened this issue Feb 2, 2017 · 7 comments

Comments

@randyzwitch
Copy link
Owner

From #179

@slin30
Copy link
Contributor

slin30 commented Feb 27, 2017

@randyzwitch, noticed CM methods are analagous to the Segments.* methods. Should be able to re-use/apply almost all segments work towards CMs. What say you to investigating and potentially extending collaboration to CMs? Am making steady progress on segments but probably need to discuss specifics, and if also tackling CMs, any decisions would likely be applicable there as well.

@randyzwitch
Copy link
Owner Author

Hi @slin30, my apologies for not getting back to you about the Segments code. Also, I hope I haven't given you or anyone else the impression I don't want to collaborate! This package is only where it is today based on contributions from a half-dozen other folks :)

Given that your Segments code is actually developed as a stand-alone package, I don't see a specific path forward to merging that code directly. However, I'd love it if you would submit a PR for Segments (and a separate one for CM if you're ambitious!). That way, we can figure out the best way forward.

Overall, your coding ability seemed far in excess of the standards in this library, so I'm not concerned about that from an internal code perspective! The main thing I would want to work with prior to merging in any new functionality is just that the public API (i.e. functions) feel similar to current code.

@slin30
Copy link
Contributor

slin30 commented Feb 28, 2017

@randyzwitch, no apologies needed--(and you're too kind-- my humble thanks!)-- have been chipping away at this over the past weeks, with constantly shifting priorities in terms of what functions to write/address, largely dictated by day-to-day needs as they arise.

I am fully on board with working together to ensure that any PR-appropriate functions maintain a consistent feel/structure within RSiteCatalyst, prior to merging. And I definitely would like/need your input when it comes to general architecture, since Segments.Get can return a diverse range of data structures (and I expect the same will be true of the corresponding CM method).

I still have a bit of work to do in terms of stitching pieces together and writing basic tests for parsing various return possibilities, but the basic call is pretty much complete. Can discuss in greater detail in the gitlab mirror or #204 as you prefer.

I anticipate that any resulting design (and formatting/style) decisions for Segments.Get would be applicable to CM, and yes, I am feeling ambitious in that regard!

@randyzwitch
Copy link
Owner Author

The best thing to do is just start pull requests here, so we can keep the discussions focused and use the line comment functionality. Thanks again for your offer for help, getting Segments and CM implemented will be a great addition!

@slin30
Copy link
Contributor

slin30 commented Mar 1, 2017

Will do, likely within a week from today, starting with Segments GET function(s).

@slin30
Copy link
Contributor

slin30 commented Mar 6, 2017

Quick initial results, based on something that came up where I could not access all the CMs associated with an RSID, even after using my full admin credentials: I can pretty much use the call.Get_Segments function as-is, simply swapping out the argument for the func.name.

Since the Adobe documentation for CalculatedMetrics.Get is quite sparse/seems to be in-progress, I need to do some more digging and testing. But the good news is that we should be able to get a 2-for-1 here, with a bit more time/work on my part to understand the similarities and refactor/streamline as sensible.

@randyzwitch randyzwitch changed the title Deprecate GetCalculatedMetrics, implement Calculated Metrics API Implement Calculated Metrics API Mar 8, 2017
@randyzwitch
Copy link
Owner Author

Related: #220

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

2 participants