-
Notifications
You must be signed in to change notification settings - Fork 3
fix: enforce name length limit #77
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
Conversation
Codecov ReportAttention: Patch coverage is
✅ All tests successful. No failed tests found.
Additional details and impacted files@@ Coverage Diff @@
## main #77 +/- ##
==========================================
- Coverage 96.95% 96.89% -0.06%
==========================================
Files 15 16 +1
Lines 1902 1931 +29
==========================================
+ Hits 1844 1871 +27
- Misses 58 60 +2 ☔ View full report in Codecov by Sentry. |
Codecov ReportAttention: Patch coverage is ✅ All tests successful. No failed tests found.
📢 Thoughts on this report? Let us know! |
CodSpeed Performance ReportMerging #77 will not alter performanceComparing Summary
|
5b97bbf
to
2f264fb
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.
This looks very reasonable, I do have a couple of suggestions:
- Creating a
fn new
to create empty strings makes sense and will clean up a bunch of code - Similarly, switching over string literals in tests to
"literal".into()
might simplify a ton of that test code. - You have a bunch of
unwrap
s still scattered around a few places where they might be hit in production. These should be fixed and rather just propagate the error.
we want to limit the length of individual testrun fields (except for failure messages). This is done for DB index limit reasons, but also because if the testrun fields exceed this length, something is probably wrong with the user's setup.
- implement tryfrom and tryinto for validated_string - replace left over unwrap calls - use pyo3 transparent to derive intopyobject and frompyobject
2f264fb
to
6b3e952
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.
All these "".try_into().unwrap()
still read really weird. A new()
/Default
for an empty string would be nice.
As well as a simpler constructor for literals.
avoid doing a bunch of "".try_into().unwrap() and just use the default
eeb75ce
to
da61d26
Compare
we want to limit the length of individual testrun fields (except for
failure messages). This is done for DB index limit reasons, but also
because if the testrun fields exceed this length, something is
probably wrong with the user's setup.