Merge lp://staging/~jeff-apple/openvista-gtm-integration/bug355710 into lp://staging/openvista-gtm-integration
Proposed by
jeff.apple
Status: | Merged | ||||
---|---|---|---|---|---|
Merged at revision: | not available | ||||
Proposed branch: | lp://staging/~jeff-apple/openvista-gtm-integration/bug355710 | ||||
Merge into: | lp://staging/openvista-gtm-integration | ||||
Diff against target: | None lines | ||||
To merge this branch: | bzr merge lp://staging/~jeff-apple/openvista-gtm-integration/bug355710 | ||||
Related bugs: |
|
Reviewer | Review Type | Date Requested | Status |
---|---|---|---|
Jon Tai | Approve | ||
Review via email: mp+5778@code.staging.launchpad.net |
To post a comment you must log in.
Needs header comment update.
The structure of this thing is odd. It enters what is essentially the "body" of the FOR loop once in the DO before starting the FOR loop, then repeats the body again with the same variables on the first FOR loop iteration. Is this the desired behavior? If not, then the DO can be removed.
I would rename the variable OS to be more specific, such as ISGTM. Actually rather than that, why not remove OS completely and have ROUT determine the platform instead of passing it as a parameter? That way it could be expanded for other platforms besides GTM and Cache.