search

openshift / origin-server

OpenShift 2 (deprecated)
889 stars 516 forks source link

deployments_controller.rb: Adds check for REST deployment without arguments #6311

Closed thrasher-redhat closed 8 years ago

thrasher-redhat commented 8 years ago

deployments_controller.rb: Adds check for REST deployment without arguments

Bug 1223025 Bugzilla: https://bugzilla.redhat.com/show_bug.cgi?id=1223025

Currently, attempting to change a deployment and not passing in a target variable (either ref or artifact_url) will result in a false success message.

Adds a check to ensure that a ref or artifact_url variable is present before attempting to deploy.

tiwillia commented 8 years ago

Other than the one note, this LGTM. We should probably have a test for it.

thrasher-redhat commented 8 years ago

Added test for this case (and another similar case that wasn't covered in testing).

tiwillia commented 8 years ago

[test]

tiwillia commented 8 years ago

LGTM, lets make sure tests pass before we merge.

Miciah commented 8 years ago

Looks good but for a grammatical error in a comment.

openshift-bot commented 8 years ago

Evaluated for online test up to 1d2ff2c539a6bfc82b6d7d9f84566232e1d89847

openshift-bot commented 8 years ago

Online Test Results: SUCCESS (https://ci.dev.openshift.redhat.com/jenkins/job/test_pull_requests/9048/)

tiwillia commented 8 years ago

[merge]

openshift-bot commented 8 years ago

Online Merge Results: SUCCESS (https://ci.dev.openshift.redhat.com/jenkins/job/merge_pull_requests/6636/) (Image: devenv_5709)

tiwillia commented 8 years ago

Please try to [merge] again, as the failures appear to be unrelated.

openshift-bot commented 8 years ago

Evaluated for online merge up to 1d2ff2c539a6bfc82b6d7d9f84566232e1d89847