jeantil commented on PR #2960: URL: https://github.com/apache/james-project/pull/2960#issuecomment-4241584375
Hello A couple comments - i don't think there is a test documenting the metadata name format - I'm aware this is not currently the case but adding comments or some kind of documentation explaining the design of BlobStoreDAO would be helpful. It always takes me quite some time to recontextualise this code. Something to explain that this is james virtual blobstore abstraction. That the bucketname is a james specific logical bucket which may be represented in various ways in actual storage connectors. In particular it should not be conflated with an S3 bucket name. Then an explanation over why metadata is exposed (use case which justified the implementation and potential use cases ) possibly explain the distinction between metadata actively used by james which has direct helpers (and thus new helpers should be added if james starts to use more metadata) vs the rest which is exposed as an extension point for users of james libraries. I know that technically this information is available in the JIRA and in this MR's discussion but this requires extra effort to discover or rediscover over having the why directly exposed in the contract. The class documentation should be kept rather high level to limit it's tendency to rot but still explain the major design choices. cheers -- 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] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
