Feature/userdel diffs #584
Conversation
| @@ -217,19 +183,10 @@ func (u *User) Check(context.Context, resource.Renderer) (resource.TaskStatus, e | |||
|
|
|||
| // Apply changes for user | |||
| func (u *User) Apply(context.Context) (resource.TaskStatus, error) { | |||
There was a problem hiding this comment.
why couldn't apply use the status that you've created already, again? I feel like we had this discussion, but a) I don't remember and b) has the answer changed?
There was a problem hiding this comment.
I don't believe we can since we no longer keep status as a field of the task struct.
| @@ -401,6 +336,40 @@ func (u *User) DiffAdd(status *resource.Status) (*AddUserOptions, error) { | |||
| return options, nil | |||
| } | |||
|
|
|||
| // DiffDel checks for differences between the current and desired state for the | |||
| // user to be deleted indicated by the User fields. | |||
| func (u *User) DiffDel(status *resource.Status, userByName *user.User, nameNotFound bool) error { | |||
There was a problem hiding this comment.
nitpick: should this be public?
There was a problem hiding this comment.
It makes sense to be private, but is public for testing purposes.
There was a problem hiding this comment.
IMO it would be better to make the tests part of the package than it would be to expose extra members. But it's definitely not a blocker. :)
There was a problem hiding this comment.
After some discussion, we've concluded to leave as is for user since testing is currently performed for a package <name> in package <name_test>. In the next resource added to Converge, we will try testing as part of the package.
| case nameNotFound: | ||
| return nil | ||
| case userByName != nil: | ||
| switch { |
There was a problem hiding this comment.
nitpick: this if … else in fancier clothes
There was a problem hiding this comment.
I thought you liked switch better. ;) I will change it back to if...else.
| } | ||
| } | ||
|
|
||
| if resource.AnyChanges(status.Differences) { |
There was a problem hiding this comment.
possible enhancement: why isn't this a method on status? (cc @rebeccaskinner)
Improve diffs used when deleting a user.
Implements #445