Skip to content
This repository has been archived by the owner on Apr 6, 2020. It is now read-only.

Library TypeScript Transition #38

Merged
merged 9 commits into from
Jan 9, 2019
Merged

Library TypeScript Transition #38

merged 9 commits into from
Jan 9, 2019

Conversation

holgerd77
Copy link
Member

TypeScript transition based on the initial work on the RLP library ethereumjs/rlp#37.

@holgerd77
Copy link
Member Author

@s1na @krzkaczor Hmm, I have now npm run test passing locally with formatting and linting, but build is failing for Prettier with all files differ, not sure how to proceed/debug. Did you experience something similar respectively have an idea?

@krzkaczor
Copy link

@holgerd77 is there a possibility that you run an old version of common prettier config package? Maybe rm -rf node_modules and reinstall :D

I am not sure if you can SSH to travis CI but this would also be a good idea (you def can do this on circeci).

@holgerd77
Copy link
Member Author

@krzkaczor Hmm, have also done (reinstall node modules).

@s1na
Copy link
Contributor

s1na commented Jan 8, 2019

@holgerd77 I had also faced this, but wasn't sure what's the cause. Just tried it in your branch and the same happens for me. Interestingly if you run npx prettier --list-different **/*.{ts,json,md} it does catch the errors!
Changing "format" command in package.json to "prettier --list-different '**/*.{ts,json,md}'" seems to solve it (note the ' ' around the glob).

@coveralls
Copy link

coveralls commented Jan 8, 2019

Coverage Status

Coverage decreased (-0.04%) to 95.902% when pulling f3e235f on typescript into bc2fa6f on master.

@holgerd77
Copy link
Member Author

@s1na Works! ✨

Thanks! 😄

@holgerd77 holgerd77 requested review from alextsg and s1na January 8, 2019 14:38
Copy link
Contributor

@s1na s1na left a comment

Choose a reason for hiding this comment

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

Looks good! There were just a few minor comments.

Another point: on Common class, return type annotations for the methods before hardforkBlock have probably been forgotten 😄

package.json Outdated Show resolved Hide resolved
src/index.ts Outdated Show resolved Hide resolved
src/types.ts Show resolved Hide resolved
@holgerd77
Copy link
Member Author

Have updated the missing return values.

@holgerd77
Copy link
Member Author

@s1na Hi Sina, just pushed, can you have another look and approve if you are satisfied with everything?

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

Successfully merging this pull request may close these issues.

4 participants