Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove mocks for transaction manager and transaction queue #562

Merged
merged 1 commit into from
Jan 26, 2018

Conversation

dshulyak
Copy link
Contributor

It is very unlikely that there will be 2 or more implementations of tx manager and queue, as they are tailored specifically to status project requirements.

Blocked by: #530

Copy link
Contributor

@divan divan left a comment

Choose a reason for hiding this comment

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

100 x 👍

We definitely need to clean up overused and autogenerated without a valid reason mocks, so I'm super happy to see this PR.

Copy link
Contributor

@themue themue left a comment

Choose a reason for hiding this comment

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

Yes, wonderful simplification and good step in dropping these autogenerated mocks.

@@ -8,17 +8,14 @@ import (

"github.com/ethereum/go-ethereum/accounts/keystore"
gethcommon "github.com/ethereum/go-ethereum/common"
"github.com/status-im/status-go/geth/account"
"github.com/status-im/status-go/geth/common"
"github.com/status-im/status-go/geth/log"
)

const (
// DefaultTxQueueCap defines how many items can be queued.
DefaultTxQueueCap = int(35)
Copy link
Contributor

Choose a reason for hiding this comment

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

Interesting. Isn't int the default here and casting only needed when explicitly wanting something like uint64 or similar.

}

// LockInprogress returns transcation and locks it as inprogress
func (q *TxQueue) LockInprogress(id common.QueuedTxID) (*common.QueuedTx, error) {
Copy link
Contributor

Choose a reason for hiding this comment

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

LockInprogress() or LockInProgress()?

@themue
Copy link
Contributor

themue commented Jan 26, 2018

@dshulyak Will you take care of the conflicts, please? Thx.

It is very unlikely that there will be 2 or more implementations
of tx manager and queue, as they are tailored specifically to status project
requirements.
@dshulyak
Copy link
Contributor Author

@themue rebased, please consider merging it

@themue themue merged commit ba0b20e into status-im:develop Jan 26, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants