add ability to specify last as build number for deploy #1956
Conversation
|
ping... |
|
LGTM |
|
my concern with this logic is that if the last build is event type |
drone/deploy.go
Outdated
| return err | ||
| } | ||
| for _, build := range builds { | ||
| if build.Status == model.StatusSuccess && build.Event != model.EventDeploy { |
There was a problem hiding this comment.
also should consider teams that may want to filter out certain branches, to avoid deploying a feature branch. In the documentation, for example, we recommend the following code for launching jobs which takes branch into account:
drone build start --fork octocat/hello-world \
$(drone build last --format="{{ .Number }}" --branch=master octocat/hello-world)
There was a problem hiding this comment.
i don't know about pull_requests , but may be good to filter it out, because in case of pr we can take exact number...
About branch suggestion - ok
There was a problem hiding this comment.
or may be simple add params for event type and branch ? so by default it uses only push event type, but if user wants it can be deploy event and pull request...? what do you think?
|
@bradrydzewski i'm change params and use sane defaults. So if user does not specify anything and use last keyword, deploy uses push event, with status success and master branch. |
|
gentle ping... (sorry i need this feature and not want to keep own repo for long time..) |
drone/deploy.go
Outdated
| cli.StringSliceFlag{ | ||
| Name: "param, p", | ||
| Usage: "custom parameters to be injected into the job environment. Format: KEY=value", | ||
| }, | ||
| }, | ||
| } | ||
|
|
||
| func eventContains(events []string, search string) bool { |
|
changes look good, just 1 question about (maybe) unused function? |
Signed-off-by: Vasiliy Tolstov <[email protected]>
|
i'm fix issue and force pushed, so this is ready to merge! Thanks! |
Signed-off-by: Vasiliy Tolstov [email protected]