Upgrade to angular 1.5 and patternfly 3.18.1 #1250
Conversation
jwforres
force-pushed
the
angular-1.5-upgrade
branch
from
1ba6f46 to
bb01b7f
Compare
jwforres
force-pushed
the
angular-1.5-upgrade
branch
from
bb01b7f to
0ee6608
Compare
|
cc @dtaylor113 @jeff-phillips-18 @spadgett started this branch that contains dave's changes plus my fixes for orderBy. I got all the other repos updated on the angular dependency so there are no bower conflicts now. |
jwforres
force-pushed
the
angular-1.5-upgrade
branch
from
0ee6608 to
7689654
Compare
|
Looks like travis is happy now |
|
LGTM! Thanks @jwforres 👍 |
|
@spadgett think this is ready for review now, particularly the second commit |
jwforres
changed the title
app/scripts/controllers/quota.js
Outdated
| @@ -102,13 +102,13 @@ angular.module('openshiftConsole') | |||
| $scope.project = project; | |||
|
|
|||
| DataService.list("resourcequotas", context, function(quotas) { | |||
| $scope.quotas = quotas.by("metadata.name"); | |||
| $scope.quotas = _.sortBy(quotas.by("metadata.name"), 'name'); | |||
There was a problem hiding this comment.
Should these be... ?
$scope.quotas = _.sortBy(quotas.by("metadata.name"), "metadata.name");There was a problem hiding this comment.
If we're doing to do this often, is it worth adding it directly to Data in DataService? Something like...? (untested)
Data.prototype.sort = function(property) {
property = property || 'metadata.name';
return _.sortBy(this.by("metadata.name"), property);
};Then
$scope.quotas = quotas.sort();There was a problem hiding this comment.
on the data service question I tend to agree, but since we are in the middle of moving the services out to the other repo, I'd rather do that separately from this PR
app/views/browse/imagestream.html
Outdated
| <tr><td colspan="5"><em>{{emptyMessage}}</em></td></tr> | ||
| </tbody> | ||
| <tbody ng-repeat="tag in tagsByName | orderBy : 'name'"> | ||
| <tbody ng-repeat="tag in tags | orderBy : 'name'"> |
There was a problem hiding this comment.
Remove the orderBy here if we're sorting in the controller.
| "angular-animate": "1.3.20", | ||
| "angular-touch": "1.3.20", | ||
| "angular-route": "1.3.20", | ||
| "es5-dom-shim": "*", |
There was a problem hiding this comment.
Concerns me a bit we have to use "*" here :/
There was a problem hiding this comment.
We effectively already were since that is what hawtio-core was asking for before, there is not other option as the es5-dom-shim repo doesn't actually have releases.
We can probably drop this dependency altogether now, I don't think any of the browsers we support now even need this. But I would rather do that independent of this set of changes so we aren't trying to figure out what caused the issue if something ends up breaking.
There was a problem hiding this comment.
Yeah, I'm fine with it, although something we might look at. Maybe we can just exclude (not for this PR)
| @@ -48,7 +48,7 @@ angular.module('openshiftConsole') | |||
| }; | |||
| } | |||
| $scope.imageStream = imageStream; | |||
| $scope.tagsByName = ImageStreamsService.tagsByName($scope.imageStream); | |||
| $scope.tags = _.sortBy(ImageStreamsService.tagsByName($scope.imageStream), 'name'); | |||
There was a problem hiding this comment.
👍 sorting in the watch callback seems better
…ang 1.5 Also update more dependencies to fix some resolution issues.
jwforres
force-pushed
the
angular-1.5-upgrade
branch
from
7689654 to
d87d27f
Compare
|
PR updated to address review feedback plus the the stuff around image stream tags we just discussed |
|
[merge] |
|
Evaluated for origin web console merge up to d87d27f |
|
Origin Web Console Merge Results: SUCCESS (https://ci.openshift.redhat.com/jenkins/job/test_pull_requests_origin_web_console/1025/) (Base Commit: 8a5a5f1) |
No description provided.