-
Notifications
You must be signed in to change notification settings - Fork 43
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
✨ refelect ID -> Id #1482
✨ refelect ID -> Id #1482
Conversation
Signed-off-by: sarthaksarthak9 <sarthaknegi908@gmail.com>
Signed-off-by: sarthaksarthak9 <sarthaknegi908@gmail.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.
The changes look good. The PR needs to be rebased to main with a merge conflict resolved. With a rebase, should be all set!
yah sure!! |
Signed-off-by: Sarthak Negi <122533767+sarthaksarthak9@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.
LGTM
Codecov ReportAll modified lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #1482 +/- ##
=======================================
Coverage 40.69% 40.69%
=======================================
Files 139 139
Lines 4463 4463
Branches 1067 1067
=======================================
Hits 1816 1816
Misses 2552 2552
Partials 95 95
Flags with carried forward coverage won't be shown. Click here to find out more.
☔ View full report in Codecov by Sentry. |
Appears that konveyor have since settled on "ID->Id". There are still a few places where we need to reflect this decision in the code. E.g. getKindIDByRef, useFetchBusinessServiceByID, etc
closes #1307