You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@brooklyn.apache.org by GitBox <gi...@apache.org> on 2021/06/04 11:12:36 UTC
[GitHub] [brooklyn-ui] iuliana opened a new pull request #212: Made sure the .config.bundle is initialized correctly and thus a corr…
iuliana opened a new pull request #212:
URL: https://github.com/apache/brooklyn-ui/pull/212
…ect URL is generated when a catalog item is added from the composer
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [brooklyn-ui] asfgit closed pull request #212: Made sure the .config.bundle is initialized correctly and thus a corr…
Posted by GitBox <gi...@apache.org>.
asfgit closed pull request #212:
URL: https://github.com/apache/brooklyn-ui/pull/212
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [brooklyn-ui] iuliana commented on a change in pull request #212: Made sure the .config.bundle is initialized correctly and thus a corr…
Posted by GitBox <gi...@apache.org>.
iuliana commented on a change in pull request #212:
URL: https://github.com/apache/brooklyn-ui/pull/212#discussion_r645538506
##########
File path: ui-modules/blueprint-composer/app/components/catalog-saver/catalog-saver.directive.js
##########
@@ -101,9 +101,10 @@ export function saveToCatalogModalDirective($rootScope, $uibModal, $injector, $f
$scope.config.version = entity.version || metadata.get('version');
}
if (!$scope.config.bundle) {
- if ($scope.config.symbolicName) {
- $scope.config.bundle = $scope.config.symbolicName;
+ if (!$scope.config.symbolicName) {
+ $scope.config.symbolicName = $scope.config.name;
Review comment:
`symbolicName` is initialized at line 98 already. - only it isn't, that is why the link is broken.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [brooklyn-ui] ahgittin commented on a change in pull request #212: Made sure the .config.bundle is initialized correctly and thus a corr…
Posted by GitBox <gi...@apache.org>.
ahgittin commented on a change in pull request #212:
URL: https://github.com/apache/brooklyn-ui/pull/212#discussion_r648117164
##########
File path: ui-modules/blueprint-composer/app/components/catalog-saver/catalog-saver.directive.js
##########
@@ -142,6 +142,8 @@ export function CatalogItemModalController($scope, $filter, blueprintService, pa
saving: false,
force: false,
};
+ $scope.catalogBundleId ="";
+ $scope.catalogBundleBase ="";
Review comment:
`""` will generate incorrect URL so not sure it's worth initializing with invalid value, but it should get populated quickly so NBD
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [brooklyn-ui] algairim commented on a change in pull request #212: Made sure the .config.bundle is initialized correctly and thus a corr…
Posted by GitBox <gi...@apache.org>.
algairim commented on a change in pull request #212:
URL: https://github.com/apache/brooklyn-ui/pull/212#discussion_r645514669
##########
File path: ui-modules/blueprint-composer/app/components/catalog-saver/catalog-saver.directive.js
##########
@@ -101,9 +101,10 @@ export function saveToCatalogModalDirective($rootScope, $uibModal, $injector, $f
$scope.config.version = entity.version || metadata.get('version');
}
if (!$scope.config.bundle) {
- if ($scope.config.symbolicName) {
- $scope.config.bundle = $scope.config.symbolicName;
+ if (!$scope.config.symbolicName) {
+ $scope.config.symbolicName = $scope.config.name;
Review comment:
There is a lot of logic tied up to `symbolicName` and to its absence. It effects the behaviour of c`omposer-blueprint-name-validator` directive a well, the absence of any tests makes it harder to see. I would not initialise `symbolicName` here but assign the `conig.name` to `config.bundle` directly, if `symbolicName` is not initialised.
`symbolicName` is initialized in couple of places already.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [brooklyn-ui] algairim commented on a change in pull request #212: Made sure the .config.bundle is initialized correctly and thus a corr…
Posted by GitBox <gi...@apache.org>.
algairim commented on a change in pull request #212:
URL: https://github.com/apache/brooklyn-ui/pull/212#discussion_r645514669
##########
File path: ui-modules/blueprint-composer/app/components/catalog-saver/catalog-saver.directive.js
##########
@@ -101,9 +101,10 @@ export function saveToCatalogModalDirective($rootScope, $uibModal, $injector, $f
$scope.config.version = entity.version || metadata.get('version');
}
if (!$scope.config.bundle) {
- if ($scope.config.symbolicName) {
- $scope.config.bundle = $scope.config.symbolicName;
+ if (!$scope.config.symbolicName) {
+ $scope.config.symbolicName = $scope.config.name;
Review comment:
There is a lot of logic tied up to `symbolicName` and to its absence. It effects the behaviour of c`omposer-blueprint-name-validator` directive a well, the absence of any tests makes it harder to see. I would not initialise `symbolicName` here but assign the `conig.name` to `config.bundle` directly, if `symbolicName` is not initialised.
`symbolicName` is initialized at line 98 already.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [brooklyn-ui] ahgittin commented on pull request #212: Made sure the .config.bundle is initialized correctly and thus a corr…
Posted by GitBox <gi...@apache.org>.
ahgittin commented on pull request #212:
URL: https://github.com/apache/brooklyn-ui/pull/212#issuecomment-857527254
LGTM - merging
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [brooklyn-ui] algairim commented on a change in pull request #212: Made sure the .config.bundle is initialized correctly and thus a corr…
Posted by GitBox <gi...@apache.org>.
algairim commented on a change in pull request #212:
URL: https://github.com/apache/brooklyn-ui/pull/212#discussion_r645514669
##########
File path: ui-modules/blueprint-composer/app/components/catalog-saver/catalog-saver.directive.js
##########
@@ -101,9 +101,10 @@ export function saveToCatalogModalDirective($rootScope, $uibModal, $injector, $f
$scope.config.version = entity.version || metadata.get('version');
}
if (!$scope.config.bundle) {
- if ($scope.config.symbolicName) {
- $scope.config.bundle = $scope.config.symbolicName;
+ if (!$scope.config.symbolicName) {
+ $scope.config.symbolicName = $scope.config.name;
Review comment:
There is a lot of logic tied up to `symbolicName` and to its absence. It effects the behaviour of c`omposer-blueprint-name-validator` directive a well, the absence of any tests makes it harder to see. I would not initialise `symbolicName` here but assign the `conig.name` to `config.bundle` directly, if `symbolicName` is not initialised.
`symbolicName` is initialized in a couple of places already.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org