-
Notifications
You must be signed in to change notification settings - Fork 1.8k
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
Refactor placement config and client out of internal #7446
base: master
Are you sure you want to change the base?
Conversation
Signed-off-by: Artur Souza <asouza.pro@gmail.com>
Signed-off-by: Artur Souza <asouza.pro@gmail.com>
Signed-off-by: Artur Souza <asouza.pro@gmail.com>
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #7446 +/- ##
==========================================
- Coverage 62.41% 62.40% -0.01%
==========================================
Files 240 238 -2
Lines 22062 22040 -22
==========================================
- Hits 13769 13755 -14
+ Misses 7153 7141 -12
- Partials 1140 1144 +4 ☔ View full report in Codecov by Sentry. |
@@ -11,7 +11,7 @@ See the License for the specific language governing permissions and | |||
limitations under the License. | |||
*/ | |||
|
|||
package internal |
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.
Please create a new package for the interfaces, don't move them inside placement
which is a concrete implementation.
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.
No problem. Will move.
@@ -11,7 +11,7 @@ See the License for the specific language governing permissions and | |||
limitations under the License. | |||
*/ | |||
|
|||
package internal |
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 file should not be in config
. Maybe more in a package like interfaces
, together with the placement interface.
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.
No problem. Will move.
Putting this on-hold for now. I might not need it. |
This pull request has been automatically marked as stale because it has not had activity in the last 60 days. It will be closed in 7 days if no further activity occurs. Please feel free to give a status update now, ping for review, or re-open when it's ready. Thank you for your contributions! |
This pull request has been automatically marked as stale because it has not had activity in the last 60 days. It will be closed in 7 days if no further activity occurs. Please feel free to give a status update now, ping for review, or re-open when it's ready. Thank you for your contributions! |
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #7446 +/- ##
==========================================
- Coverage 62.41% 62.40% -0.01%
==========================================
Files 240 238 -2
Lines 22062 22040 -22
==========================================
- Hits 13769 13755 -14
+ Misses 7153 7141 -12
- Partials 1140 1144 +4 ☔ View full report in Codecov by Sentry. |
Description
Refactor placement config and client out of internal package to allow control plane service to also connect to placement service.
No behavior change.
Issue reference
Please reference the issue this PR will close: N/A
Checklist
Please make sure you've completed the relevant tasks for this PR, out of the following list: