sipa / bech32

Code snippets and analysis of the Bech32 format
191 stars 106 forks source link

Rust Implementation #12

Closed clarkmoody closed 7 years ago

clarkmoody commented 7 years ago

Bech32 encoding of SegWit addresses in a Rust Crate.

Instructions are in README

sipa commented 7 years ago

Cool, is this ready? I'll find someone to review the code then (unfortunately I do not know Rust yet).

clarkmoody commented 7 years ago

I may change the API, so I'll update the PR when it's ready for review. Needs more docs too :-)

On Sat, Apr 1, 2017, 11:19 Pieter Wuille notifications@github.com wrote:

Cool, is this ready? I'll find someone to review the code then (unfortunately I do not know Rust yet).

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/sipa/bech32/pull/12#issuecomment-290930198, or mute the thread https://github.com/notifications/unsubscribe-auth/ABfXMdpwgA3dfqFpWauQRczusykHMdMsks5rrnkEgaJpZM4Mtpv- .

clarkmoody commented 7 years ago

The Rust library is ready for review.

apoelstra commented 7 years ago

Hi, I can take a look at this.

apoelstra commented 7 years ago

Can you squash the first three commits and the wit_prof rename into one commit?

clarkmoody commented 7 years ago

I don't think I can put the license data commit (a55881c) after the rename commit (f086b45).

How about a squash to three commits total?

  1. First three commits: Initial implementation and tests passing
  2. Add license and package data
  3. Last three commits: Rename + refactor API into final version
apoelstra commented 7 years ago

Sounds good to me!

clarkmoody commented 7 years ago

Commits pushed.

apoelstra commented 7 years ago

On a plane in about 8 hours, will be out of commission for a day or two while I adjust to the timezone. But I haven't forgotten about this :)

apoelstra commented 7 years ago

ACK except for

Otherwise this looks great!

clarkmoody commented 7 years ago

@apoelstra Did you make comments beyond commit squash above? Those may have been lost.

I've brought my fork up to sipa:master in the meantime, removed a few trailing spaces, and added a simple README.

apoelstra commented 7 years ago

Can you see my comments now? I didn't realize github had added several extra button clicks to comment.

clarkmoody commented 7 years ago

Thanks, I see them now. Looks like the comments were for a previous commit of the code, but most of them are still applicable.

apoelstra commented 7 years ago

ACK.

@sipa do you have any opinion on whether these six commits should be squashed into one?

clarkmoody commented 7 years ago

Merged in changes from sipa:master

sipa commented 7 years ago

Yes, I prefer a clean commit history. No need to squash everything into one if there are logically separable parts, but no merges if possible.

clarkmoody commented 7 years ago

Well it looks like I made a mess of the commits by merging in sipa:master changes along the way. What should I do now?

sipa commented 7 years ago

I can squash them for you if you want.

clarkmoody commented 7 years ago

That would be great, thanks :smile:

sipa commented 7 years ago

@clarkmoody See #15.

sipa commented 7 years ago

Superseded by #15.