Refactored daemon-scheduler environment and deployment structure. #185
Conversation
@@ -282,7 +283,7 @@ func (suite *APITestSuite) TestDeleteEnvironmentMissingEnvironment() { | |||
assert.Equal(suite.T(), http.StatusNotFound, responseRecorder.Code) | |||
} | |||
|
|||
func (suite *APITestSuite) assertSame(environment *types.Environment, environmentModel *models.Environment) { | |||
func (suite *APITestSuite) assertSame(environment *environmenttypes.Environment, environmentModel *models.Environment) { |
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.
Shouldn't it be environmentTypes? camelCase?
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.
Yes for variables. No for packages. environmenttypes is a package, not a variable. You'll see we reference package names in all lowercase everywhere.
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.
Oh I see
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.
Minor comment. You can address it in the deployments refactor if you want.
inProgressLock sync.RWMutex | ||
id string | ||
ctx context.Context | ||
environmentService environment.EnvironmentService |
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.
Can we make this environmentSvc
to keep it consistent with deploymentSvc
declared below? Or vice versa (use ..Service
for both).
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.
Yup, I'll rename deploymentSvc
to deploymentService
when I do the deployments refactor.
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!
Summary
Refactored daemon-scheduler environment and deployment structure.
Implementation details
Testing
cd cluster-state-service; make; cd ../
)cd cluster-state-service; make release; cd ../
)cd daemon-scheduler; make; cd ../
)cd daemon-scheduler; make release; cd ../
)New tests cover the changes: no
Description for the changelog
Refactored daemon-scheduler environment and deployment structure.
Licensing
This contribution is under the terms of the Apache 2.0 License: Yes
Before merging