-
Notifications
You must be signed in to change notification settings - Fork 46
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
Merge bcp demo into weave #91
Conversation
… swap (#37) * Enforce escrow sender and recipient are addresses, not conditions * Handler and model make proper address/condition checks * Update all handler logic to new format
WIP: Begin roadmap for q3 work
Codecov Report
@@ Coverage Diff @@
## master #91 +/- ##
=======================================
- Coverage 77.5% 76.5% -1%
=======================================
Files 62 80 +18
Lines 2531 3399 +868
=======================================
+ Hits 1962 2601 +639
- Misses 462 627 +165
- Partials 107 171 +64
Continue to review full report at Codecov.
|
@@ -0,0 +1,91 @@ | |||
# bov |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We can rename to bcpd
here
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great. I was wondering about naming...
This will break some of the Docker scripts as well, so maybe they can fix that up once we have a new name.
@@ -0,0 +1,91 @@ | |||
# bov | |||
|
|||
Blockchain of value reference implementation |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Blockchain Communications Protocol Daemon...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
maybe do bov >>> bcpd rename in a second PR. the rest looks good text wise.
Codecov Report
@@ Coverage Diff @@
## master #91 +/- ##
=======================================
- Coverage 77.5% 76.5% -1%
=======================================
Files 62 80 +18
Lines 2531 3399 +868
=======================================
+ Hits 1962 2601 +639
- Misses 462 627 +165
- Partials 107 171 +64
Continue to review full report at Codecov.
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
- Code merge looks very good! 🌷
- Travis build needs some updates to work proper but let's do this in a new PR
- there are still some references to
bcp-demo
in the.rts
files that can be done in a new PR
🚀
@@ -1,7 +1,19 @@ | |||
# Binaries for programs and plugins |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
fyi: You can have a global .gitignore: https://github.com/alpe/dotfiles/blob/master/.gitignore
@@ -0,0 +1,51 @@ | |||
sudo: required |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The .travis.yml
file needs to go into the project root folder. But let me fix this in a new PR afterwards. There are also travis-ci settings for docker hub to be configured in order to push the artifact.
cmd/bcpd/commands/docs.go
Outdated
@@ -0,0 +1,8 @@ | |||
/* | |||
Package commands has integration tests of the mycoind |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
q: should mycoind
be replaced with bcpd?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yup
cmd/bcpd/app/tx.go
Outdated
|
||
// we must have covered it above | ||
panic(sum) | ||
// return nil, errors.ErrUnknownTxType(nil) // alpe???? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
pp: ja, don't panic but return ErrUnknownTxType :-)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You got the message. Will do
One repo for all the code. This gets rid of lots of headaches
x/*
for reusable extensions andexamples/bov
for app codeOnce this is basically working, we can start improving the code quality and reuse in other PRs
CI updates are done separately in #93