Use FileVault Package Maven Plugin 1.3.0 #2804
Conversation
kwin
force-pushed
the
feature/use-filevault-package-maven-plugin
branch
3 times, most recently
from
02cde7e to
2d17ee5
Compare
Codecov Report
@@ Coverage Diff @@
## master #2804 +/- ##
=========================================
Coverage 53.81% 53.81%
Complexity 5539 5539
=========================================
Files 753 753
Lines 30476 30476
Branches 3938 3938
=========================================
Hits 16400 16400
Misses 12546 12546
Partials 1530 1530 Continue to review full report at Codecov.
|
kwin
force-pushed
the
feature/use-filevault-package-maven-plugin
branch
4 times, most recently
from
3f5e8cb to
df1d18d
Compare
kwin
force-pushed
the
feature/use-filevault-package-maven-plugin
branch
3 times, most recently
from
3d6f579 to
69fe07b
Compare
kwin
force-pushed
the
feature/use-filevault-package-maven-plugin
branch
from
69fe07b to
dbfa5f9
Compare
|
@davidjgonzalez Do you have time to review this? |
| @@ -50,7 +50,7 @@ angular.module('acs-commons-qr-code-app', ['acsCoral', 'ACS.Commons.notification | |||
| $http({ | |||
| method: 'POST', | |||
| url: $scope.app.uri + '/config', | |||
| data: 'enabled=' + $scope.form.enabled || 'false', | |||
| data: 'sling:resourceType=acs-commons/components/utilities/qr-code/config&enabled=' + $scope.form.enabled || 'false', | |||
There was a problem hiding this comment.
This doesn't look like a related change, was is intended to be included?
There was a problem hiding this comment.
Necessary because the config at "/etc/acs-commons/qr-code/jcr:content/config" should not be be overwritten (as the enabled flag should survive installations) but at the same time the resource type is necessary for rendering json (unfortunate mix of code and content). As FileVault only supports property filtering in later versions, the whole config node is now never overwritten and its resource type set with every config change.
| @@ -305,7 +305,6 @@ | |||
| sling:resourceType="granite/ui/components/coral/foundation/panel/railpanel"/> | |||
| </rails> | |||
| </jcr:content> | |||
| <createlistwizard/> | |||
There was a problem hiding this comment.
Was this intended to be removed?
There was a problem hiding this comment.
Empty nodes are only for ordering purposes, and as that node is not there in the first place (in the best case) this will lead to validation issues with FileVault and is therefore consequently be removed here.
| @@ -5,10 +5,5 @@ | |||
| jcr:primaryType="cq:PageContent" | |||
| jcr:title="QR Code" | |||
| sling:resourceType="acs-commons/components/utilities/qr-code"> | |||
| <config | |||
| <exclude pattern="/apps/acs-commons/components/utilities/version-compare/clientlibs/css/generated"/> | ||
| <exclude pattern="/apps/acs-commons/config(\..*)?(/.*)?" /> |
There was a problem hiding this comment.
@kwin i missed this one originally - so OSGi configs are now moved into a content folder in the content (all) package? Is this idiomatic? I thought typically these should be in a ui.config package using the "regular" packaging methods?
There was a problem hiding this comment.
This is the recommended layout: https://jackrabbit.apache.org/filevault/packagetypes.html
There was a problem hiding this comment.
Gotcha - but alternatively we could also put them in a ui.config page that is a container type - you just added them to this project/package since this is the existing container type project we have (vs. making a new one).
There was a problem hiding this comment.
That would be possible as well, but requires a new Maven module. I don’t particularly like the AEM project archetype lay-out as ui.config is not specific enough as name. There are more configurations than OSGi configurations (e.g. context aware configuration)
There was a problem hiding this comment.
Yup - understood. Just wanted to check if there was something special about putting them in our equivalent of "all". Thanks!
This closes #2781 and #2043