-
Notifications
You must be signed in to change notification settings - Fork 33
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
Drop invocation reasons #1412
Drop invocation reasons #1412
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #1412 +/- ##
==========================================
+ Coverage 86.15% 86.83% +0.67%
==========================================
Files 218 216 -2
Lines 6457 6357 -100
==========================================
- Hits 5563 5520 -43
+ Misses 894 837 -57 ☔ View full report in Codecov by Sentry. |
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.
Good work! One thing on the migration only.
priv/repo/migrations/20231120112129_remove_invocation_reasons_and_constraints.exs
Outdated
Show resolved
Hide resolved
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.
hey @benedictus-dev , looking great. quick things:
- please update the changelog
- I still see both
InvocationReason
andInvocationReasons
in here. Should those files be deleted? If not, why not? - In one of your changes to the
attempt_service_test
, I noticed this line. Should we drop this test (and the corresponding function from the attempt service?)
This isn't an exhaustive review, but would love for you to address these things before asking for a review from Stu.
Touch up tests and remove (with their tests): - Lightning.AttemptService - Lightning.Workflows.Graph (replaced by Lightning.Graph) Closes: #1412
24eb157
to
bf6b246
Compare
Notes for the reviewer
Related issue
Invocation reasons are no longer be generated. This PR drops the table from the DB and remove all unused code in the InvocationReason module.
Fixes #1402
Review checklist