Re: AW: PR-33: problems

2017-05-29 Thread Gintautas Grigelionis 
Sorry for slow response, I'm on a trip. It does make sense to leave the
exception in place only if ivy becomes opinionated about versions. I am not
sure if compatibility break requires a change of major version. There are
semver tools out there; maybe it's worth integrating them (redemption
driven development :-)

Gintas

Den 29 maj 2017 11:47 skrev "Jan Matèrne (jhm)" :

Sounds like I would do ;)
I'll merge the PR locally and insert the delegates.

Open is
"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."

https://wiki.eclipse.org/Evolving_Java-based_APIs_2 defines the removal of
a checked exception:
"Breaks compatibility"
"Adding and deleting checked exceptions declared as thrown by an API method
  does not break binary compatibility; however,
  it breaks contract compatibility (and source code compatibility)."

What do we want?



Jan

> -Ursprüngliche Nachricht-
> Von: J Pai [mailto:jai.forums2...@gmail.com]
> Gesendet: Montag, 29. Mai 2017 10:26
> An: Ant Developers List
> Betreff: Re: PR-33: problems
>
> IMO, for each of these public classes/methods/fields that we are fixing
> for typos, we should mark them @Deprecated (and add a @deprecated
> javadoc too and point to the new field/method/class) and introduce the
> rightly named class/method/field. For methods it’s straightforward, the
> deprecated method internally calls the new method. For fields too it’s
> straightforward. The deprecated field uses the value of the new field.
> For classes, I think we can copy over the existing class into the new
> one and leave around the existing one just for possible external
> references (that we can’t control off) and migrate all internal
> references to the new one.
>
> At some point, in some future version of Ivy, we remove/delete these
> deprecated method/field/class.
>
>
> -Jaikiran
>
>
> On 29-May-2017, at 1:43 PM, Jan Matèrne  wrote:
>
> I did a review of  
> 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.
>
>
>
>
>
>
>
>
>
>
>
> -
> To unsubscribe, e-mail: dev-unsubscr...@ant.apache.org For additional
> commands, e-mail: dev-h...@ant.apache.org



-
To unsubscribe, e-mail: dev-unsubscr...@ant.apache.org
For additional commands, e-mail: dev-h...@ant.apache.org

AW: PR-33: problems

2017-05-29 Thread jhm 
Sounds like I would do ;)
I'll merge the PR locally and insert the delegates.

Open is
"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."

https://wiki.eclipse.org/Evolving_Java-based_APIs_2 defines the removal of a 
checked exception:
"Breaks compatibility"
"Adding and deleting checked exceptions declared as thrown by an API method 
  does not break binary compatibility; however, 
  it breaks contract compatibility (and source code compatibility)."

What do we want?



Jan

> -Ursprüngliche Nachricht-
> Von: J Pai [mailto:jai.forums2...@gmail.com]
> Gesendet: Montag, 29. Mai 2017 10:26
> An: Ant Developers List
> Betreff: Re: PR-33: problems
> 
> IMO, for each of these public classes/methods/fields that we are fixing
> for typos, we should mark them @Deprecated (and add a @deprecated
> javadoc too and point to the new field/method/class) and introduce the
> rightly named class/method/field. For methods it’s straightforward, the
> deprecated method internally calls the new method. For fields too it’s
> straightforward. The deprecated field uses the value of the new field.
> For classes, I think we can copy over the existing class into the new
> one and leave around the existing one just for possible external
> references (that we can’t control off) and migrate all internal
> references to the new one.
> 
> At some point, in some future version of Ivy, we remove/delete these
> deprecated method/field/class.
> 
> 
> -Jaikiran
> 
> 
> On 29-May-2017, at 1:43 PM, Jan Matèrne  wrote:
> 
> I did a review of  
> 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.
> 
> 
> 
> 
> 
> 
> 
> 
> 
> 
> 
> -
> To unsubscribe, e-mail: dev-unsubscr...@ant.apache.org For additional
> commands, e-mail: dev-h...@ant.apache.org



-
To unsubscribe, e-mail: dev-unsubscr...@ant.apache.org
For additional commands, e-mail: dev-h...@ant.apache.org

AW: PR-33: problems

2017-05-29 Thread jhm 
Thanks, but I already have it done ;)

But one point is open:
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.


Jan

> -Ursprüngliche Nachricht-
> Von: Gintautas Grigelionis [mailto:g.grigelio...@gmail.com]
> Gesendet: Montag, 29. Mai 2017 11:00
> An: Ant Developers List
> Betreff: Re: PR-33: problems
> 
> If it's acceptable I'll complement the PR addressing all the points.
> 
> Gintas
> 
> Den 29 maj 2017 10:13 skrev "Jan Matèrne" :
> 
> I did a review of  
> 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.


-
To unsubscribe, e-mail: dev-unsubscr...@ant.apache.org
For additional commands, e-mail: dev-h...@ant.apache.org