lecle / aloxide

Aloxide is a pragmatic abstraction layer for various blockchain softwares.
Apache License 2.0
10 stars 5 forks source link

feat: add jest config to make codecoverage for abstraction package #59

Closed sanglt1902 closed 4 years ago

sanglt1902 commented 4 years ago
tiendq commented 4 years ago

@sanglt1902 I still don't see some issues resolved, at least I don't know how it is resolved. Added more comments.

It hard to review your PR again, please don't mark the comment as resolved if it isn't resolved by code changes (I see old code but it marked resolved then I don't know how it is resolved). I don't know how to unresolve too.

And don't push force, I can't see what is the difference between new changes and old code. You should keep small commits so other can easily track the issue. When everything is done well and ready to merge you can squash to make a nice commit log.

I think it should be something all we should follow to make reviewing easier, so I cc you 👍

/cc @manh-vv @hoang-dao @quocle108 @danielAlvess

sanglt1902 commented 4 years ago

@sanglt1902 I still don't see some issues resolved, at least I don't know how it is resolved. Added more comments.

It hard to review your PR again, please don't mark the comment as resolved if it isn't resolved by code changes (I see old code but it marked resolved then I don't know how it is resolved). I don't know how to unresolve too.

And don't push force, I can't see what is the difference between new changes and old code. You should keep small commits so other can easily track the issue. When everything is done well and ready to merge you can squash to make a nice commit log.

I think it should be something all we should follow to make reviewing easier, so I cc you 👍

/cc @manh-vv @hoang-dao @quocle108 @danielAlvess

Thank you so much!

tiendq commented 4 years ago

@sanglt1902 LGTM.