-
Notifications
You must be signed in to change notification settings - Fork 281
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
ci(coverage): add total typescript code coverage statistics #3285
ci(coverage): add total typescript code coverage statistics #3285
Conversation
af3d1d1
to
759faae
Compare
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.
@aldousalvarez Thank you very much for this! It's looking very good so far, I just have some questions above^^
a775e73
to
f53931f
Compare
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.
@aldousalvarez Please see #3285 (comment)
f53931f
to
ee4f0c8
Compare
Hello @petermetz already done with the requested changes. Thank you. It will not run on this pull request but it will run on the default branch which is main once it gets merged. |
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.
Hello @petermetz already done with the requested changes. Thank you. It will not run on this pull request but it will run on the default branch which is main once it gets merged.
@aldousalvarez Thank you for the updates! I agree it's time for us to take a look at it and see if it works on main
. Fingers crossed! :-)
LGTM too |
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.
@aldousalvarez Please fix the merge conflicts and then we are good to go! (Make sure to pass it back for review once the conflicts are fixed)
ee4f0c8
to
bf3853b
Compare
@petermetz Already fixed the merge conflict. Thank you! |
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.
@aldousalvarez It might've happened again with another PR in the meantime, please fix again :-)
Primary Changes ---------------- 1. Updated the ci.yaml and ci.sh to introduce total typescript code coverage statistics using jest and istanbul-merge 2. Added Codecov to cspell.json Fixes hyperledger#2661 Signed-off-by: aldousalvarez <[email protected]>
bf3853b
to
1deb63f
Compare
Hello @petermetz just fixed the merged conflicts. Thank you |
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.
@aldousalvarez LGTM, thank you!
Commit to be reviewed
ci(coverage): add total typescript code coverage statistics
Fixes #2661
Pull Request Requirements
upstream/main
branch and squashed into single commit to help maintainers review it more efficient and to avoid spaghetti git commit graphs that obfuscate which commit did exactly what change, when and, why.-s
flag when usinggit commit
command. You may refer to this link for more information.Character Limit
A Must Read for Beginners
For rebasing and squashing, here's a must read guide for beginners.