-
Notifications
You must be signed in to change notification settings - Fork 1
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
feature/add types checking #53
base: develop
Are you sure you want to change the base?
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## develop #53 +/- ##
========================================
Coverage 99.30% 99.30%
========================================
Files 4 4
Lines 286 286
Branches 53 53
========================================
Hits 284 284
Misses 2 2 ☔ View full report in Codecov by Sentry. |
scripts/common.ts
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
fix type-checking issues
} | ||
}, | ||
"include": ["config/**/*", "deploy/**/*", "scripts/**/*","test/**/*"], | ||
"exclude": ["node_modules", "typechain-types"], |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There is no way to exclude libs/IexecLibOrders_v5.ts
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, great questions! I spent a lot of time investigating this point. This is what I initially thought as well.
However, when you include a file, all imports from that file are also checked, along with the files imported by those child files, and so on recursively. Only files from typechain-types that are not imported by the included files will not be tested, which helps save time.
Normally, we don’t worry about nested files being tested because, as they are imported from a library, they should already be correct. However, in the case of libs/IexecLibOrders_v5.ts
, there is clearly an issue. This might be due to the use of a very old Solidity compiler, but I’m not entirely sure.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks :)
@@ -2,6 +2,7 @@ | |||
|
|||
## vNEXT | |||
|
|||
- Add type-checking script |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
add the PR number :)
No description provided.