If it's acceptable I'll complement the PR addressing all the points. Gintas
Den 29 maj 2017 10:13 skrev "Jan Matèrne" <j...@materne.de>: I did a review of <https://github.com/apache/ant-ivy/pull/33> https://github.com/apache/ant-ivy/pull/33 Here are the points I have problems with, so I want to discuss them here. Basically it's about breaking BC. So how to deal with that? Jan Fixing the spell error in DelegateHandler$ChildElementHandler (s/childHanlded/childHandled/) means breaking beakward compatiblity. We could introduce a delegetate for that: /** for BC */ @Deprecated public void childHanlded(DH child) throws SAXParseException { childHandled(DH child); } While refactoring you have renamed all occurences in the Ivy codebase. On the other hand I don't know the impact (maybe outside of Ivy). I'll bring that to the dev-list. src/java/org/apache/ivy/osgi/repo/FSManifestIterable.java: renaming the public constant DEFAULT_BUNLDE_FILTER also means breaking BC. src/java/org/apache/ivy/osgi/util/Version.java: the constructor removes the (IMO unneccessary) ParseException. But because it is a checked Exception we break BC. renaming EncrytedProperties to EncryptedProperties means breaking BC. If required we could introduce a delegating class or a subclass. ArtifactOrigin: renaming unkwnown() to unknown() means breaking BC. If required we could introduce a delegating method.
