wchevreuil edited a comment on pull request #3721: URL: https://github.com/apache/hbase/pull/3721#issuecomment-937818470
> The TestStoreFileTracker.testHasPersistConfiguration method is used to make sure that all the implementation classes have the correct static method. My problem with this approach is that we then don't have an intuitive interface that clearly states what should be implemented, requiring this extra hack to enforce its rules, at test execution time only. I would rather have a more strict interface that it's easier for developer to understand what is required to be implemented (the original approach), or, since this seems to be an issue exclusively for the FILE SFT impl, let it deal with the config mode on its own (the extra check for missing info in the passed StoreContext implemented on last commit). WDYT? > For the naming change, updateDescriptor seems too general, what is the reason for changing the method name? I do not mean we can not change it, just want to know the reason. While going through this, I thought the `persistConfiguration` confusing. Even though I was the author of this method on few commits back, now whenever I was reading it, I was misled to think it was persisting the configuration somewhere, when in reality it was just updating the passed descriptor builder with additional configs needed by individual SFT impl. As it seems `updateDescriptor` still sounds confusing, let me try something else. How about `updateWithTrackerConfigs`? -- 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]
