-
Notifications
You must be signed in to change notification settings - Fork 64
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
Dev Notes: Add section on E2E tests #610
Conversation
@anubh-v Not sure if this is too in-depth or is missing any crucial info. Is the structure of it okay as well? |
Codecov Report
@@ Coverage Diff @@
## master #610 +/- ##
=======================================
Coverage 67.20% 67.20%
=======================================
Files 71 71
Lines 2150 2150
Branches 199 199
=======================================
Hits 1445 1445
+ Misses 664 663 -1
- Partials 41 42 +1
Continue to review full report at Codecov.
|
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.
Dev notes are very helpful 👍
I think I picked up a few spelling errors, and having a newline above headings would make it more readable without preview :)
Other than thanks for the hard work!
Sure! Haha I think the syntax highlighting lit up the headers for me so I didn't think about adding spaces. I'll put them in for clarity 👍 |
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 for adding the section to Dev Notes!:) LGTM other than the minor grammatical mistakes and typo!👍
Thanks for helping catch the errors! 👍 |
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.
LGTM 👍
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 for working on this. It looks good overall, but let's reorder the sections and use more descriptive section headings.
Co-authored-by: Anubhav <35621759+anubh-v@users.noreply.github.com>
Co-authored-by: Anubhav <35621759+anubh-v@users.noreply.github.com>
Co-authored-by: Anubhav <35621759+anubh-v@users.noreply.github.com>
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.
Some comments specific to the first two sections.
Thanks alot for writing this up!
Co-authored-by: Anubhav <35621759+anubh-v@users.noreply.github.com>
Co-authored-by: Anubhav <35621759+anubh-v@users.noreply.github.com>
Co-authored-by: Anubhav <35621759+anubh-v@users.noreply.github.com>
Co-authored-by: Anubhav <35621759+anubh-v@users.noreply.github.com>
Co-authored-by: Anubhav <35621759+anubh-v@users.noreply.github.com>
Co-authored-by: Anubhav <35621759+anubh-v@users.noreply.github.com>
Co-authored-by: Anubhav <35621759+anubh-v@users.noreply.github.com>
Co-authored-by: Anubhav <35621759+anubh-v@users.noreply.github.com>
Co-authored-by: Anubhav <35621759+anubh-v@users.noreply.github.com>
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.
Looks good, just one final fix. Thanks for the work
Fixes #606
Proposed Commit Message: