rmannibucau commented on code in PR #34: URL: https://github.com/apache/geronimo-xbean/pull/34#discussion_r1055299704
########## xbean-finder/src/test/java/org/apache/xbean/finder/archive/JarArchiveTest.java: ########## @@ -16,15 +16,10 @@ */ package org.apache.xbean.finder.archive; -import org.acme.foo.Blue; Review Comment: Here the goal is to reduce the patch size and side effect of a global IDE shortcut for other dev, nothing related to disk or repository infrastructure. Long story short the rational is: 1. limit the number of lines touched by the patch to the needed ones (once again no issue to drop useless imports but re-sorting it is a bad practise IMHO and breaks 2) 2. ensure that blaming the class we don't get the last person who sorted the import which kind of breaks the blame feature. We want the actual person who coded the line last and when I say coded here, I mean with some IP and not just some IDE refactoring. Both are important and this is why I'm asking you to comply to the existing style - note: I didn't pick it myself and I'm not judging it is good or bad ;), just to ensure it works in time. In terms of sorting, you picked alphabetical one but there are other common strategies which are valid: * java then javax then others then static * static then java then javax then others * java then others * alphabetical * ... None is bad and none is worse than another. Generally speaking when you contribute to a project you respect its style you like it or not - and I'm the first one to complain about some ASF style ;) - so please just ensure git diff is minimal and highlights the change you did without side effects. -- 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: dev-unsubscr...@geronimo.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
