Skip to content
This repository was archived by the owner on Aug 24, 2021. It is now read-only.

Cleanup and more methods #11

Merged
merged 4 commits into from
May 14, 2016
Merged

Cleanup and more methods #11

merged 4 commits into from
May 14, 2016

Conversation

dignifiedquire
Copy link
Member

No description provided.

@coveralls
Copy link

coveralls commented May 12, 2016

Coverage Status

Coverage increased (+25.9%) to 100.0% when pulling 99a0317 on rework into 4487064 on master.

return new Buffer(s, 'hex')
}

exports.toB58String = function toB58String (m) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we've been using the toSomething as obj.toNewFormat. Since this is a utility function and not something available on the object, I would prefer and argue it would be less confusing if instead of 'to' we use the word 'convert' or 'transform'

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

that would make these very long, e.g.

multihash.convertToB58String(myBuffer)
// compared to
multihash.toB58String(myBuffer)

Because you call it on the module I fell it's pretty clear that you convert from multihash to

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ok then

@dignifiedquire
Copy link
Member Author

@diasdavid also added api docs :)


- `code: Number`

Checks wether a code is part of the app range.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What is an appCode?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Anything between 0 and 0x10. I'm not a 100% sure where in the spec they are defined

@daviddias
Copy link
Member

LGTM :)

@dignifiedquire dignifiedquire merged commit 92f6c06 into master May 14, 2016
@dignifiedquire dignifiedquire deleted the rework branch May 14, 2016 12:16
@daviddias
Copy link
Member

released 0.2.2 :)

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.

3 participants