-
Notifications
You must be signed in to change notification settings - Fork 156
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
Copy pkg/diff to pkg/plugin/diff except DiffStructureds and RenderByCommand #5297
Conversation
Signed-off-by: Shinnosuke Sawada-Dazai <[email protected]>
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #5297 +/- ##
==========================================
+ Coverage 24.46% 25.03% +0.56%
==========================================
Files 444 447 +3
Lines 47024 47520 +496
==========================================
+ Hits 11506 11896 +390
- Misses 34602 34687 +85
- Partials 916 937 +21 ☔ View full report in Codecov by Sentry. |
@Warashi, I got your point 👍 (1) Same as we discussed before, should this be placed under |
@khanhtc1202 |
Signed-off-by: Shinnosuke Sawada-Dazai <[email protected]>
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, thank you 👍
@Warashi |
@t-kikuc |
What this PR does:
as title.
Why we need it:
We want to use the diff package to implement plugins, so I copied them under the plugin package.
Discussion point:
pkg/diff
from plugin implementations directly?I thought we should not use
pkg/diff
directly from plugin implementations, so I copied them under the plugin package to represent the package as a prepared utility for plugins.I thought we should copy the smallest functionalities set at this time, so I copied not all of the package, but it became almost all. I copied the whole except DiffStructureds and RenderByCommand.
Which issue(s) this PR fixes:
Part of #4980
Does this PR introduce a user-facing change?: