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

tooling: adding script to inspect TS interface diffs #764

Merged
merged 2 commits into from
Jun 8, 2020

Conversation

evertonfraga
Copy link
Contributor

@evertonfraga evertonfraga commented Jun 4, 2020

My goal is to quickly inspect what interfaces have changed since the packages' latest release. This is specially useful for releases.

Instead of going over diff for the source files (which would be chaotic), I chose to perform diff on the .d.ts files. They are generated at build time, therefore not part of this repo, so I can't use git to compare such files.

The ts-interface-diff.sh script downloads the latest published package, and compares the .d.ts files recursively to the ones found in the local dist directory, printing the diff to stdout. I made it generic enough to be used with any other package, so feel free to test it elsewhere, if you want.

Basic instructions on the script file itself.

How to test this PR

  1. make sure to have dist files for the package you want to inspect
# install deps, links packages
npm i 

# build all packages
lerna run build
  1. Run script
# Run script
./scripts/ts-interface-diff.sh ethereumjs-vm packages/vm/ 

Real-life output:

125c105
<     getContractStorage(address: Buffer, key: Buffer, cb: any): void;
---
>     getContractStorage(address: Buffer, key: Buffer): Promise<Buffer>;
134c114
<     getOriginalContractStorage(address: Buffer, key: Buffer, cb: any): void;
---
>     getOriginalContractStorage(address: Buffer, key: Buffer): Promise<Buffer>;
156c134
<     clearContractStorage(address: Buffer, cb: any): void;
---
>     clearContractStorage(address: Buffer): Promise<void>;

@codecov
Copy link

codecov bot commented Jun 4, 2020

Codecov Report

Merging #764 into master will decrease coverage by 0.01%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #764      +/-   ##
==========================================
- Coverage   92.06%   92.05%   -0.02%     
==========================================
  Files          46       44       -2     
  Lines        3025     3021       -4     
  Branches      471      471              
==========================================
- Hits         2785     2781       -4     
  Misses        145      145              
  Partials       95       95              
Flag Coverage Δ
#account 94.11% <ø> (ø)
#block 88.36% <ø> (ø)
#blockchain 89.15% <ø> (ø)
#common 93.52% <ø> (-0.26%) ⬇️
#tx 94.16% <ø> (+0.13%) ⬆️
#vm 93.10% <ø> (ø)
Impacted Files Coverage Δ
packages/common/src/genesisStates/index.ts
packages/common/src/chains/index.ts
packages/common/src/hardforks/index.ts
packages/tx/src/index.ts 100.00% <0.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 2b8b314...d33e4a7. Read the comment docs.

@alcuadrado
Copy link
Member

This is really cool, @evertonfraga ! 👏🏻

@holgerd77
Copy link
Member

This is really cool 😄 , please do a rebase and drop a note once you have change the diff command and this is ready for re-review, so that this won't get forgotten.

@evertonfraga
Copy link
Contributor Author

Thanks @holgerd77!

I am changing some control flows to make a continuous git diff, instead of a separate diffs that you have to be quitting one by one.

# cleanup
rm -rf $TGZ $CACHE_PATH

git diff --no-index -- $A_PATH $B_PATH
Copy link
Contributor Author

Choose a reason for hiding this comment

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

To fulfill the ultimate diff experience, a single diff command should be run at the end, instead of separate ones. This gave north to every other change, and the script was basically entirely rewritten.

Copy link
Member

Choose a reason for hiding this comment

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

Great idea!

@evertonfraga evertonfraga requested a review from alcuadrado June 5, 2020 11:51
@evertonfraga
Copy link
Contributor Author

This is ready for review :)

Copy link
Contributor

@ryanio ryanio left a comment

Choose a reason for hiding this comment

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

nice, just tested out the script it's working great for me!

@evertonfraga evertonfraga merged commit 47e8ca1 into master Jun 8, 2020
evertonfraga added a commit that referenced this pull request Jun 10, 2020
* tooling: adding script to inspect TS interface diffs

* tooling: refactoring script to improve usability
@evertonfraga evertonfraga added this to the VM v5 milestone Jun 12, 2020
@evertonfraga evertonfraga deleted the d-ts-files-diff branch October 14, 2020 01:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants