search

multiformats / js-cid

CID implementation in JavaScript
MIT License
97 stars 39 forks source link

feat: add ts types with aegir #131

Closed hugomrdias closed 3 years ago

hugomrdias commented 4 years ago

this is a best effort approach since multiformats/multiformats will probably superseed this repo soon.

Gozala commented 3 years ago

It also removes the withIs, which is a (potentially) huge breaking change. I'd prefer if we keep that in for compatibility. I don't think it makes sense to remove it as it will be replaced by multiformats/cid anyway.

@vmx withIs unfortunately triggers various different bugs in TS & I'm guessing that is also why it end up been part of this PR. Switch to multiformats/cid might be larger effort but probably worth considering. I'd be interested in helping with that effort.

hugomrdias commented 3 years ago

@vmx im just trying to keep these repos consistent with each other and has you said we might move to the new multiformats soon, so theres no point in spending too much time in this and thats why i just added everything in.

vmx commented 3 years ago

@Gozala @achingbrain @hugomrdias So we do a new major release of js-cid for adding TypeScript types? If there's agreement that's fine with me.

@hugomrdias Though next time I'd prefer having things like adding issue templates in a separate commit as it's totally unrelated.

hugomrdias commented 3 years ago

@vmx nothing is actually a breaking change it behaves exactly as before, just a little bit safer now

vmx commented 3 years ago

@Gozala @achingbrain @hugomrdias: oh sorry I missed that line: https://github.com/multiformats/js-cid/pull/131/files#diff-bfe9874d239014961b1ae4e89875a6155667db834a410aaaa2ebe3cf89820556R17

Which add the symbol which withIs() is adding. So it's not a breaking change.

vmx commented 3 years ago

I've removed the issue templates, I don't want them in this repo, I think it increases the burden of reporting a bug without having too much benefit (we also don't get that many bug reports here).