Skip to content
This repository has been archived by the owner on May 3, 2022. It is now read-only.

Update action.Action interface #687

Merged
merged 1 commit into from Apr 11, 2019

Conversation

silvin-lubecki
Copy link
Contributor

All the action implementations added a 3rd argument io.Writer, but this change was never reflected to the actual interface action.Action.
I also added static guards to check the actions implements the interface.

Signed-off-by: Silvin Lubecki silvin.lubecki@docker.com

@silvin-lubecki
Copy link
Contributor Author

The linter is failing on master too, here is the PR fixing it #688
We should merge the fix before this one.

@silvin-lubecki
Copy link
Contributor Author

@radu-matei I think I don't have access to the logs of the duffle-ci check. Can you please take a look?

@@ -13,6 +13,9 @@ type Install struct {
Driver driver.Driver // Needs to be more than a string
}

// makes sure Install implements Action interface
var _ Action = &Install{}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we used to put this check in _test.go files, but I'm not sure this is consistent across the entire code base.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I moved them to their respective test files 👍

@silvin-lubecki silvin-lubecki force-pushed the update-action-interface branch 2 times, most recently from e8d78dd to 39ebb61 Compare April 8, 2019 15:57
@silvin-lubecki
Copy link
Contributor Author

PTAL @radu-matei @michelleN

Copy link
Member

@radu-matei radu-matei left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

Add static guards to check the actions implements the interface.

Signed-off-by: Silvin Lubecki <silvin.lubecki@docker.com>
@silvin-lubecki
Copy link
Contributor Author

Rebased on top of master.

@radu-matei radu-matei merged commit 9c16b9e into cnabio:master Apr 11, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants