{"action":"created","issue":{"url":"https://api.github.com/repos/kubernetes/dashboard/issues/680","repository_url":"https://api.github.com/repos/kubernetes/dashboard","labels_url":"https://api.github.com/repos/kubernetes/dashboard/issues/680/labels{/name}","comments_url":"https://api.github.com/repos/kubernetes/dashboard/issues/680/comments","events_url":"https://api.github.com/repos/kubernetes/dashboard/issues/680/events","html_url":"https://github.com/kubernetes/dashboard/pull/680","id":151106947,"number":680,"title":"WIP: Expose external","user":{"login":"cheld","id":7448799,"avatar_url":"https://avatars.githubusercontent.com/u/7448799?v=3","gravatar_id":"","url":"https://api.github.com/users/cheld","html_url":"https://github.com/cheld","followers_url":"https://api.github.com/users/cheld/followers","following_url":"https://api.github.com/users/cheld/following{/other_user}","gists_url":"https://api.github.com/users/cheld/gists{/gist_id}","starred_url":"https://api.github.com/users/cheld/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/cheld/subscriptions","organizations_url":"https://api.github.com/users/cheld/orgs","repos_url":"https://api.github.com/users/cheld/repos","events_url":"https://api.github.com/users/cheld/events{/privacy}","received_events_url":"https://api.github.com/users/cheld/received_events","type":"User","site_admin":false},"labels":[{"url":"https://api.github.com/repos/kubernetes/dashboard/labels/cla:%20yes","name":"cla: yes","color":"bfe5bf"}],"state":"open","locked":false,"assignee":null,"milestone":null,"comments":2,"created_at":"2016-04-26T11:16:46Z","updated_at":"2016-04-26T14:04:05Z","closed_at":null,"pull_request":{"url":"https://api.github.com/repos/kubernetes/dashboard/pulls/680","html_url":"https://github.com/kubernetes/dashboard/pull/680","diff_url":"https://github.com/kubernetes/dashboard/pull/680.diff","patch_url":"https://github.com/kubernetes/dashboard/pull/680.patch"},"body":"Once again, not intended to be merged. I would like to get opinion. It kind of works, but it is just hacked. Code would need clean up and tests.\r\n\r\nI refactored the code, but I am not sure, is it good or for bad?\r\n\r\nPositive: moving the expose service checkbox into the directive. \r\nNegative: removing the watch requires to add direct function calls at any place the model might be changed. There would be still one watch to be removed and the code would get even more complicated, I assume.\r\n\r\nSomehow, I feel we are doing something wrong. \r\n\r\nIMHO, the best option is to keep the checkbox in the directive but add a watch again. Or we think about handling the validation differently?\r\n\r\n@maciaszczykm  can you please have a look and tell me your opinion?\r\n\r\n<!-- Reviewable:start -->\r\n---\r\nThis change is [<img src=\"https://reviewable.io/review_button.svg\" height=\"35\" align=\"absmiddle\" alt=\"Reviewable\"/>](https://reviewable.io/reviews/kubernetes/dashboard/680)\r\n<!-- Reviewable:end -->\r\n"},"comment":{"url":"https://api.github.com/repos/kubernetes/dashboard/issues/comments/214755849","html_url":"https://github.com/kubernetes/dashboard/pull/680#issuecomment-214755849","issue_url":"https://api.github.com/repos/kubernetes/dashboard/issues/680","id":214755849,"user":{"login":"bryk","id":1159999,"avatar_url":"https://avatars.githubusercontent.com/u/1159999?v=3","gravatar_id":"","url":"https://api.github.com/users/bryk","html_url":"https://github.com/bryk","followers_url":"https://api.github.com/users/bryk/followers","following_url":"https://api.github.com/users/bryk/following{/other_user}","gists_url":"https://api.github.com/users/bryk/gists{/gist_id}","starred_url":"https://api.github.com/users/bryk/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/bryk/subscriptions","organizations_url":"https://api.github.com/users/bryk/orgs","repos_url":"https://api.github.com/users/bryk/repos","events_url":"https://api.github.com/users/bryk/events{/privacy}","received_events_url":"https://api.github.com/users/bryk/received_events","type":"User","site_admin":false},"created_at":"2016-04-26T14:04:05Z","updated_at":"2016-04-26T14:04:05Z","body":"\n\n\n\nReview status: 0 of 3 files reviewed at latest revision, 2 unresolved discussions, some commit checks failed.\n\n---\n\n*[src/app/frontend/deploy/deployfromsettings.html, line 119 \\[r1\\]](https://reviewable.io:443/reviews/kubernetes/dashboard/680#-KGHrp_7oyXU776vAbj2:-KGHrp_7oyXU776vAbj3:2068333911) ([raw file](https://github.com/kubernetes/dashboard/blob/8eaf1909f61a955b49b1af708428664fff26bc30/src/app/frontend/deploy/deployfromsettings.html#L119)):*\nHere you can add a custom validator:\n```html\n<md-checkbox ng-model=\"ctrl.isExternal\" class=\"md-primary\"\n                 ng-model-options=\"{ debounce: { 'default': 500, 'blur': 0 } }\"\n                 kd-exposed-with-port-mappings=\"ctrl.portMappings\">\n```\nAnd have your validator\n\n```js\nangular.module('foo').directive('kd-exposed-with-port-mappings', function() {\n  return {\n    restrict: 'E',\n    require: 'ngModel',\n    scope: {'kd-exposed-with-port-mappings': '='},\n    link: function(scope, elem, attrs, ngModelController) {\n       validate(scope.kdExposedWithPortMappings, ngModelController);\n    }\n  }\n});\n```\n\n@maciaszczykm @cheld WDYT? \n\n---\n\n*[src/app/frontend/deploy/portmappings_controller.js, line 116 \\[r1\\]](https://reviewable.io:443/reviews/kubernetes/dashboard/680#-KGHrFVpi9GSR9uVoGC1:-KGHrFVpi9GSR9uVoGC2:1071723521) ([raw file](https://github.com/kubernetes/dashboard/blob/bde4489936d2fa557b2eb0b11712ac832fffae5f/src/app/frontend/deploy/portmappings_controller.js#L116)):*\n`$$` properties are angular _private_ things. You should not be using them as they may change at any time. \n\n---\n\n\n*Comments from [Reviewable](https://reviewable.io:443/reviews/kubernetes/dashboard/680)*\n<!-- Sent from Reviewable.io -->\n"}}