anchela commented on pull request #99:
URL: 
https://github.com/apache/sling-org-apache-sling-feature-cpconverter/pull/99#issuecomment-906337523


   @karlpauls , a couple of observations regarding the patch and 
VaultPackageAssembler in general
   - it seems really wrong that  VaultPackageAssembler implements EntryHandler
   - even worse there are several methods to add or create a particular entry 
all of which are public and used across the code base..... in other words: 
adding the extra path handling in one method might just cover one particular 
case and be missed by other cases where another add-entry method is called by 
some other handler.... this not going to work.
   - the path handling in cp-converter module is really brittle IMHO and most 
test-packages are too trivial to catch the edge cases. there should be a 
consistent naming in place to make sure it's always clear if a given path is 
actually a JCR path or a path pointing to a file in the package which has 
special semantics.... i wouldn't be surprised if there more bugs hidden in the 
code base.


-- 
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.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


Reply via email to