Skip to content

Conversation

@shea256
Copy link
Contributor

@shea256 shea256 commented Apr 7, 2014

Made the grammar a bit more consistent, cleared up the checksum verification in DecodeBase58Check, and added a short note above the declaration of pszBase58 clarifying the characters that are omitted.

@laanwj
Copy link
Member

laanwj commented Apr 7, 2014

If you're updating the comments here anyway, it'd be nice if you changed them so that those function documentations are picked up by doxygen (it currently shows no doc for them at all https://dev.visucore.com/bitcoin/doxygen/base58_8h.html#a2a7a6efa38bda9181b9a28ab3e675bea ).

You can do this by using javadoc-style docblocks above the functions (or one of the shortened forms):

/**
 * ... text ...
 */

@shea256
Copy link
Contributor Author

shea256 commented Apr 8, 2014

OK perfect, I'll make that update.

@shea256
Copy link
Contributor Author

shea256 commented Apr 8, 2014

Just pushed a commit with javadoc-style docblocks. How does that look?

@laanwj
Copy link
Member

laanwj commented Apr 9, 2014

Looks good to me, ACK after squashing into one commit

@fanquake
Copy link
Member

@rxl Any chance of get these squashed?

update comments so doxygen will pick them up
@shea256
Copy link
Contributor Author

shea256 commented Apr 12, 2014

Done.

@BitcoinPullTester
Copy link

Automatic sanity-testing: PASSED, see http://jenkins.bluematt.me/pull-tester/4e9667b844ac1c5fbb8934ec63fe17d2757f8767 for binaries and test log.
This test script verifies pulls every time they are updated. It, however, dies sometimes and fails to test properly. If you are waiting on a test, please check timestamps to verify that the test.log is moving at http://jenkins.bluematt.me/pull-tester/current/
Contact BlueMatt on freenode if something looks broken.

@fanquake
Copy link
Member

ACK

@laanwj laanwj merged commit 4e9667b into bitcoin:master Apr 19, 2014
laanwj added a commit that referenced this pull request Apr 19, 2014
4e9667b Improve and expand base58 comments (rxl)
@bitcoin bitcoin locked as resolved and limited conversation to collaborators Sep 8, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants