hemantk-12 commented on PR #5844: URL: https://github.com/apache/ozone/pull/5844#issuecomment-1866163664
> Thanks @hemantk-12 for working on this. Mostly looks good, I only noticed one small problem in `TestSecureOzoneCluster`. Also suggested a small set of related minor improvents in `TestRootedOzoneFileSystem`. > > The change is rather large, and has several different improvements. To keep patches smaller and more focused, I suggest the following (for the future): > > * When you find an opportunity for refactoring (e.g. converting to `@ParameterizedTest` to reduce duplication), create a follow-up task. Assign it to yourself, if only to avoid other eager contributors jumping on it right away (which can cause conflicts in patches). > > * For minor updates, like formatting changes, there are two possible routes: > > * make various changes in a limited set of files > * make the same limited change in many files > > Either of these make the review cycle quicker (easier to provide feedback, easier to address comments). I think the first option is better for two reasons: working in parallel with others now, and backporting such changes in the future. Thanks for the detailed suggestion. I'll keep it in mind and try to raise smaller and cosine patches in future. -- 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]
