-
Notifications
You must be signed in to change notification settings - Fork 521
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
chore: add pr template #2234
chore: add pr template #2234
Conversation
Codecov Report
@@ Coverage Diff @@
## master #2234 +/- ##
============================================
- Coverage 68.64% 65.03% -3.62%
Complexity 977 977
============================================
Files 498 498
Lines 40681 40681
Branches 5681 5681
============================================
- Hits 27927 26457 -1470
- Misses 10055 11597 +1542
+ Partials 2699 2627 -72 see 71 files with indirect coverage changes 📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
- [ ] Trivial rework / code cleanup without any test coverage. (No Need) | ||
- [ ] Already covered by existing tests, such as *(please modify tests here)*. | ||
- [ ] Need tests and can be verified as follows: | ||
- xxx |
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.
keep "None"?
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.
I don't quite understand, could you explain in more detail? :)
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.
mean that we prefer to replace "xxx" with "None" since we may not need to test manually and modify the default list.
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.
mean that we prefer to replace "xxx" with "None" since we may not need to test manually and modify the default list.
None
seems duplicate with the first option (No Need), and devs could remove it manually, we could test & enhance it later (listen the users feedback)
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.
Fine to me
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
- [ ] Trivial rework / code cleanup without any test coverage. (No Need) | ||
- [ ] Already covered by existing tests, such as *(please modify tests here)*. | ||
- [ ] Need tests and can be verified as follows: | ||
- xxx |
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.
mean that we prefer to replace "xxx" with "None" since we may not need to test manually and modify the default list.
refer: apache/incubator-hugegraph-computer#242