> On Dec. 5, 2014, 12:53 p.m., Andrii Tkach wrote: > > ambari-admin/src/main/resources/ui/admin-web/app/scripts/controllers/stackVersions/StackVersionsListCtrl.js, > > line 97 > > <https://reviews.apache.org/r/28736/diff/1/?file=783448#file783448line97> > > > > Better to use local variable and then set final array to $scope.stacks, > > because changes to $scope.stacks affect DOM.
I think this usage is OK under the context it's in. This callback function does not trigger any network based asynchronization which means angular $digest loop won't kick in during the execution. I double checked this in angular doc at https://docs.angularjs.org/guide/scope end of the page. It's a good suggestion to modify variable under $scope in local copy. For a reference type variable, unless we make a deep clone of it, holding it with another reference won't separate the actual object from been $watched. > On Dec. 5, 2014, 12:53 p.m., Andrii Tkach wrote: > > ambari-admin/src/main/resources/ui/admin-web/app/scripts/services/StackVersions.js, > > line 83 > > <https://reviews.apache.org/r/28736/diff/1/?file=783450#file783450line83> > > > > Repository version should take value of Version Name input, not a > > combination of stack and version. I haven't finished the auto population of stack version prefix in repo version input. I'm planning to make repo version input hold only subversion of the repo which means it only hold 0.1-882 when the whole repo version is 2.2.0.1-882, that's why I concatenate stack and version. After I finish everything, repo version input will be disabled first and then prepopulated with stack version after user select the stack. User will not be able to alter stack version prefix, they can only add sub version to it like shown below: Stack 2.2 Version Name: 2.2[<user-input>] > On Dec. 5, 2014, 12:53 p.m., Andrii Tkach wrote: > > ambari-admin/src/main/resources/ui/admin-web/app/scripts/services/StackVersions.js, > > line 84 > > <https://reviews.apache.org/r/28736/diff/1/?file=783450#file783450line84> > > > > I guess display name should be arbitrary label entered by user This is according to design PPT. > On Dec. 5, 2014, 12:53 p.m., Andrii Tkach wrote: > > ambari-admin/src/main/resources/ui/admin-web/app/scripts/services/StackVersions.js, > > line 97 > > <https://reviews.apache.org/r/28736/diff/1/?file=783450#file783450line97> > > > > The > > /api/v1/stacks/HDP/versions/2.2/operating_systems?fields=repositories/* > > already contains properties of repositories such as "repo_id" and > > "repo_name", only "base_url" could be modified This is what we do currentlhy for repo_id as we agreed in the meeting. - Richard ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/28736/#review63982 ----------------------------------------------------------- On Dec. 6, 2014, 12:19 a.m., Richard Zang wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/28736/ > ----------------------------------------------------------- > > (Updated Dec. 6, 2014, 12:19 a.m.) > > > Review request for Ambari, Andrii Tkach and Yusaku Sako. > > > Bugs: AMBARI-8554 > https://issues.apache.org/jira/browse/AMBARI-8554 > > > Repository: ambari > > > Description > ------- > > Ambari View > Versions > Versions table + Registration E2E integration > > > Diffs > ----- > > > ambari-admin/src/main/resources/ui/admin-web/app/assets/data/cluster/clusters.json > PRE-CREATION > > ambari-admin/src/main/resources/ui/admin-web/app/scripts/controllers/stackVersions/StackVersionsCreateCtrl.js > ae984b8 > > ambari-admin/src/main/resources/ui/admin-web/app/scripts/controllers/stackVersions/StackVersionsListCtrl.js > 698bc6c > > ambari-admin/src/main/resources/ui/admin-web/app/scripts/services/Cluster.js > c474918 > > ambari-admin/src/main/resources/ui/admin-web/app/scripts/services/StackVersions.js > ae499cc > > ambari-admin/src/main/resources/ui/admin-web/app/views/stackVersions/create.html > 11e954f > > ambari-admin/src/main/resources/ui/admin-web/app/views/stackVersions/list.html > 97e67c9 > > Diff: https://reviews.apache.org/r/28736/diff/ > > > Testing > ------- > > Manually tested on live cluster. No unit test applicable. > > > Thanks, > > Richard Zang > >
