-
Notifications
You must be signed in to change notification settings - Fork 558
Conversation
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: Assign the PR to them by writing The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
b07f2d4
to
b69da7b
Compare
Codecov Report
@@ Coverage Diff @@
## master #3084 +/- ##
==========================================
+ Coverage 51.85% 51.87% +0.01%
==========================================
Files 99 102 +3
Lines 15193 15197 +4
==========================================
+ Hits 7879 7883 +4
Misses 6590 6590
Partials 724 724 |
"light" 😛 |
b69da7b
to
f65a114
Compare
@jim-minter @pweil- For your consideration. There is some shared surface area here and will wait for your sign-off. This PR is of the "eat your spinach" variety: a little bit of tech-debt payback within the confines of an easily reviewable PR. (basically the length of a flight between PDX and SEA, which is not long enough to allow for too much change :) ) |
f65a114
to
742f534
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.
lgtm
- breakout params to params.go - k8s-specific in params_k8s.go - breakout template generator to template_generator.go - move constants to const.go - move type definitions to types.go - make unnecessary exportable properties non-exportable
6d5d672
to
4b355c4
Compare
As much as I wanted to take on this, I don't think it was possible for someone else than you guys to tackle it. Awesome 🚀 |
What this PR does / why we need it:
acsengine
package topkg/acsengine/const.go
pkg/acsengine/types.go
func
placements inengine.go
(exportable at top, private below)pkg/acsengine/params.go
andpkg/acsengine/params_k8s.go
(broken out fromengine.go
)pkg/acsengine/template_generator.go
(broken out fromengine.go
)The obvious practical benefits are to reduce
engine.go
by ~1700 lines and to make unit test // TODO work a little more approachable.There is no functional intent of this PR, any such observations would disqualify it.
Which issue this PR fixes (optional, in
fixes #<issue number>(, fixes #<issue_number>, ...)
format, will close that issue when PR gets merged): fixes #Special notes for your reviewer:
If applicable:
Release note: