Skip to content
This repository has been archived by the owner on Sep 2, 2023. It is now read-only.

Check and log transactions (RT-3499 - RT-3501) #2591

Merged
merged 5 commits into from
Sep 24, 2015

Conversation

h0vhannes
Copy link
Contributor

No description provided.

@h0vhannes h0vhannes changed the title [TASK] Assemble all RTB API calls into API service Log transactions (RT-3499) Sep 14, 2015
@h0vhannes h0vhannes changed the title Log transactions (RT-3499) Check and log transactions (RT-3499 - RT-3501) Sep 16, 2015
return $http.put(Options.backend_url + '/api/subscription', subscription, httpOptions);
};

rpAPI.addTransaction = function(transaction, result, hash, local_time) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wouldn't logTransaction be a better name for this?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

add is used to keep CRUD logic, log sounds better for sure.

@vhpoet
Copy link
Contributor

vhpoet commented Sep 16, 2015

The naming convention for angular dependencies is

  • prepend $ if the dependency is an angular service
  • don't prepend $ if the dependency is a custom service

so $scope, $routeParams, api, id, network

@h0vhannes
Copy link
Contributor Author

Totally agree with this one. I was just confused because this convention is not preserved everywhere, so that's why I used $api in a number of places. But I'll change that.

@h0vhannes
Copy link
Contributor Author

@vhpoet - fixed, please have a look

@MatthewPhinney
Copy link
Contributor

LGTM

MatthewPhinney added a commit that referenced this pull request Sep 24, 2015
Check and log transactions (RT-3499 - RT-3501)
@MatthewPhinney MatthewPhinney merged commit 638b7d4 into ripple:develop Sep 24, 2015
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants