Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: adds typescript types + type tests #110

Merged
merged 2 commits into from
Jan 27, 2020
Merged

feat: adds typescript types + type tests #110

merged 2 commits into from
Jan 27, 2020

Conversation

carsonfarmer
Copy link
Contributor

@carsonfarmer carsonfarmer commented Jan 22, 2020

Depends on multiformats/js-cid#102 for tests to run/work nicely, but this PR is already useful on its own if you don't want to use the tests in CI etc.

Signed-off-by: Carson Farmer <carson.farmer@gmail.com>
@carsonfarmer
Copy link
Contributor Author

Keeping this as a draft for now, until multiformats/js-cid#102 is (hopefully merged and) released (tho this PR is already useful as is). If you'd rather not have these types in this repo, please let me know @vasco-santos, and I can PR them to definitelytyped instead.

@carsonfarmer
Copy link
Contributor Author

See libp2p/js-libp2p-crypto#161 for a discussion of the possible applications of these types for CI etc.

Signed-off-by: Carson Farmer <carson.farmer@gmail.com>
@carsonfarmer
Copy link
Contributor Author

Type checking/tests could be added to CI by adding:

    - stage: types
      script:
        - npm run test:types

@carsonfarmer carsonfarmer marked this pull request as ready for review January 24, 2020 18:36
Copy link
Member

@hacdias hacdias left a comment

Choose a reason for hiding this comment

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

LGTM.

Btw, I see we're adding the types to the repository itself. So, if you want to move the js-multicoded to its repository, please feel free to PR and ping me 😄

@carsonfarmer
Copy link
Contributor Author

Awesome, any chance we could get a patch release once this is merged 🙏?

P.S. I will indeed ping you if/when I get a chance to update js-multicodec 👍.

@hacdias
Copy link
Member

hacdias commented Jan 27, 2020

@carsonfarmer thanks. Pinging @vasco-santos since I don't have publish permissions here.

Copy link
Contributor

@jacobheun jacobheun left a comment

Choose a reason for hiding this comment

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

LGTM 👍

@jacobheun jacobheun merged commit a5070ae into libp2p:master Jan 27, 2020
@jacobheun
Copy link
Contributor

Released in 0.13.7.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants