search

lbryio / lbrycrd

The blockchain that provides the digital content namespace for the LBRY protocol
https://lbry.com
MIT License
2.57k stars 178 forks source link

Fixes #226 #228

Closed arrodriguez closed 5 years ago

BrannonKing commented 6 years ago

I apologize that we have a silly formatting requirement at the moment. Can you run this command and submit one more time?

git diff -U0 master -- '*.h' '*.cpp' | ./contrib/devtools/clang-format-diff.py -p1 -i
BrannonKing commented 6 years ago

I would like to ask for one additional favor on this: can you (temporarily, as an experiment) modify CClaimTrieCache::removeFromExpirationQueue such that it prints to stderr whenever it succeeds to call erase and whenever it fails? I would like to know that it always succeeds in our unit tests (unless the test was made specifically to perturb the situation). It might also be good to audit the CClaimTrieCache::removeClaim to see if it returns false only in expected situations in the unit tests.

arrodriguez commented 6 years ago

Hi Thank you for your review. Before sending the pull request i execute all travis stages (for linux) . The problem is the version of clang-format version in my OS is 3.8 and the trusty version is 3.4. I think that command output is different between version. I really dont know why. I will try again in a container.

On Tue, Oct 23, 2018 at 12:58 PM Brannon King notifications@github.com wrote:

I apologize that we have a silly formatting requirement at the moment. Can you run this command and submit one more time?

git diff -U0 master -- '.h' '.cpp' | ./contrib/devtools/clang-format-diff.py -p1 -i

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/lbryio/lbrycrd/pull/228#issuecomment-432305071, or mute the thread https://github.com/notifications/unsubscribe-auth/AFbK8IRaTcfICEEjyjjlkBbjKy00cS1yks5unzyygaJpZM4X0deX .

BrannonKing commented 5 years ago

@arrodriguez I want to merge this today. Any luck with the formatting? Can you rebase on current master? In addition, I would like your formatting changes to only affect the areas specifically modified for this story.

arrodriguez commented 5 years ago

Hi @BrannonKing i will try to make the changes today.

Thanx for the patience

arrodriguez commented 5 years ago

Hi @BrannonKing , i think that all is covered here now. I did also rebase master branch with this PR.

Any feedback is welcome.

Regards,

tzarebczan commented 5 years ago

hey @arrodriguez, thanks so much for this PR and congrats on getting it merged into our codebase! If you haven't already, check out our contributing guide.

Also, can we offer you some LBC in appreciation for your contribution?

We'll also get you a special LBRY hacktoberfest t-shirt after you reach out :)

arrodriguez commented 5 years ago

Yes , please i would like to receive some LBC for the contribution.

Here is my address:

bRu6UAubBgfNUTrPyeYyeGLNE6q4kHVFUP

i will try to do more contributions whenever i can.

Thanx a lot!

tzarebczan commented 5 years ago

@arrodriguez awesome, thanks for providing that info. Can you please send us an email to hello@lbry.io per our appreciation process?