Skip to content
This repository has been archived by the owner on Feb 12, 2024. It is now read-only.

fix: use class is function on ipns #1617

Merged
merged 4 commits into from
Oct 29, 2018
Merged

Conversation

vasco-santos
Copy link
Member

@vasco-santos vasco-santos commented Oct 4, 2018

As instance of fails using different versions of a module, it was changed to use class-is.

Needs:

Fixes #1615

@ghost ghost assigned vasco-santos Oct 4, 2018
@ghost ghost added the status/in-progress In progress label Oct 4, 2018
Copy link
Contributor

@fsdiogo fsdiogo left a comment

Choose a reason for hiding this comment

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

LGTM 👍

Copy link
Member

@lidel lidel left a comment

Choose a reason for hiding this comment

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

LGTM

I tested this with ipfs/interface-datastore#24 in the context of ipfs-companion (#1615) and the fix works as expected, datastore key does not have a valid format error is gone.

@alanshaw alanshaw changed the title fix: use class is function on ipns [WIP] fix: use class is function on ipns Oct 15, 2018
Copy link
Member

@alanshaw alanshaw left a comment

Choose a reason for hiding this comment

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

@vasco-santos
Copy link
Member Author

vasco-santos commented Oct 17, 2018

@alanshaw the peerId instance is obtained through js-ipfs/core/ipns/publisher.js#L33. Considering that _putRecordToRouting is a private function js-ipfs/core/ipns/publisher.js#L53 maybe we should remove this validation. The previous tests did not hit those lines, as well.

I am not able to test this, unless I test the private functions directly or mock the PeerId.createFromPrivKey function. However, if I mock it, it will stop immediately in the first case that is hit, and the others will not be covered.

@vasco-santos vasco-santos force-pushed the fix/use-class-is-function-on-ipns branch from da0df48 to d222981 Compare October 18, 2018 11:05
@vasco-santos vasco-santos changed the title [WIP] fix: use class is function on ipns fix: use class is function on ipns Oct 24, 2018
@alanshaw alanshaw merged commit c240d49 into master Oct 29, 2018
@ghost ghost removed the status/in-progress In progress label Oct 29, 2018
@alanshaw alanshaw deleted the fix/use-class-is-function-on-ipns branch October 29, 2018 09:55
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.

5 participants