akankshajain18 commented on pull request #24: URL: https://github.com/apache/sling-org-apache-sling-resourceresolver/pull/24#issuecomment-748125573
> @akankshajain18 The latest changes look pretty good! Thanks for hanging in there! I think all the concurrency and atomicity aspects are correct now. > > I added some comments with minor nits and suggestions - none are blockers IMHO. > > And I noticed that there is some inconsistency in the terminology. We have `resource_resolver_allowed_alias_locations`, `getAllowedAliasPaths`, `getOptimizedAliasResolutionAllowList`, `aliasPathAllowList` and maybe more. I guess this is due to the many rounds of refactoring. It would be nice to go over the naming again and to make it consistent. I would opt for variants of "allowedAliasLocations", which aligns everything with the naming of the configuration value's name. In my experience this makes it easier for another developer (or yourself) to read and understand the code in the future. > @akankshajain18 The latest changes look pretty good! Thanks for hanging in there! I think all the concurrency and atomicity aspects are correct now. > > I added some comments with minor nits and suggestions - none are blockers IMHO. > > And I noticed that there is some inconsistency in the terminology. We have `resource_resolver_allowed_alias_locations`, `getAllowedAliasPaths`, `getOptimizedAliasResolutionAllowList`, `aliasPathAllowList` and maybe more. I guess this is due to the many rounds of refactoring. It would be nice to go over the naming again and to make it consistent. I would opt for variants of "allowedAliasLocations", which aligns everything with the naming of the configuration value's name. In my experience this makes it easier for another developer (or yourself) to read and understand the code in the future. All suggested changes are included in the update PR. ---------------------------------------------------------------- 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: [email protected]
