-
Notifications
You must be signed in to change notification settings - Fork 7
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
feat: Add support for the new Pre-Translate Efficiency Report #65
feat: Add support for the new Pre-Translate Efficiency Report #65
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #65 +/- ##
==========================================
- Coverage 98.20% 98.17% -0.02%
==========================================
Files 66 66
Lines 2934 2944 +10
==========================================
+ Hits 2881 2890 +9
- Misses 29 30 +1
Partials 24 24 ☔ View full report in Codecov by Sentry. |
Question for the main contributors -- I observed that there aren't any negative test cases. Would it be worth adding? (ex: https://github.com/crowdin/crowdin-api-client-go/pull/65/files#diff-6aff06001ee42c02818cd8fd62715899b7c539d3bf582ee9a750e93147317d56R110 ) |
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.
@jameskim0987 thank you!
@vorobeyme what do you think about negative test cases? |
This test function There are already some negative test cases. These are cases where @jameskim0987 if you have another type of negative test case in mind, could you please provide more details? |
@vorobeyme I think checking if the required fields are missing is fine. I was thinking more in terms of input validation. For example,
From the API docs example, it looks like for |
The docs say this field As for the tests, of course, the more possible cases covered, the better. Feel free to proceed. |
@vorobeyme Thanks for your input. I will remove the |
@jameskim0987 thanks a lot for your contributions! 🚀 |
Resolves #57
Changes made in this PR:
PreTranslateEfficiencySchema
struct and its respectiveValidateSchema()
ReportPreTranslateEfficiency