You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@ant.apache.org by Jan Matèrne <ja...@materne.de> on 2017/05/29 08:13:13 UTC

PR-33: problems

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.

 

 

 

 


Re: PR-33: problems

Posted by Matt Sicker <bo...@gmail.com>.
Removing a checked exception can affect source compatibility, for example,
if that particular exception is caught in a try block and nothing else in
the try block can throw the exception. Or at least that's how I remember it
I think.

On 29 May 2017 at 06:05, Nicolas Lalevée <ni...@hibnet.org> wrote:

>
> > Le 29 mai 2017 à 11:46, Jan Matèrne (jhm) <ap...@materne.de> a écrit :
> >
> > 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?
>
> Thanks for the link, I’ll bookmark it.
>
> Then I am OK with that. The Ivy jar can still be a upgraded without
> worries, and if somebody compile against it, then he has the source so he
> can modify them.
>
> I am OK with that also because having stricter compatibility rules will be
> painful considering the wide API Ivy is exposing.
>
> Nicolas
>
> >
> >
> >
> > Jan
> >
> >> -----Ursprüngliche Nachricht-----
> >> Von: J Pai [mailto:jai.forums2013@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 <ja...@materne.de> wrote:
> >>
> >> 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.
> >>
> >>
> >>
> >>
> >>
> >>
> >>
> >>
> >>
> >>
> >>
> >> ---------------------------------------------------------------------
> >> To unsubscribe, e-mail: dev-unsubscribe@ant.apache.org For additional
> >> commands, e-mail: dev-help@ant.apache.org
> >
> >
> >
> > ---------------------------------------------------------------------
> > To unsubscribe, e-mail: dev-unsubscribe@ant.apache.org
> > For additional commands, e-mail: dev-help@ant.apache.org
> >
>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@ant.apache.org
> For additional commands, e-mail: dev-help@ant.apache.org
>
>


-- 
Matt Sicker <bo...@gmail.com>

Re: PR-33: problems

Posted by Nicolas Lalevée <ni...@hibnet.org>.
> Le 29 mai 2017 à 11:46, Jan Matèrne (jhm) <ap...@materne.de> a écrit :
> 
> 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?

Thanks for the link, I’ll bookmark it.

Then I am OK with that. The Ivy jar can still be a upgraded without worries, and if somebody compile against it, then he has the source so he can modify them.

I am OK with that also because having stricter compatibility rules will be painful considering the wide API Ivy is exposing.

Nicolas

> 
> 
> 
> Jan
> 
>> -----Ursprüngliche Nachricht-----
>> Von: J Pai [mailto:jai.forums2013@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 <ja...@materne.de> wrote:
>> 
>> 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.
>> 
>> 
>> 
>> 
>> 
>> 
>> 
>> 
>> 
>> 
>> 
>> ---------------------------------------------------------------------
>> To unsubscribe, e-mail: dev-unsubscribe@ant.apache.org For additional
>> commands, e-mail: dev-help@ant.apache.org
> 
> 
> 
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@ant.apache.org
> For additional commands, e-mail: dev-help@ant.apache.org
> 


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


Re: AW: PR-33: problems

Posted by Gintautas Grigelionis <g....@gmail.com>.
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)" <ap...@materne.de>:

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.forums2013@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 <ja...@materne.de> wrote:
>
> 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.
>
>
>
>
>
>
>
>
>
>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@ant.apache.org For additional
> commands, e-mail: dev-help@ant.apache.org



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

AW: PR-33: problems

Posted by "Jan Matèrne (jhm)" <ap...@materne.de>.
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.forums2013@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 <ja...@materne.de> wrote:
> 
> 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.
> 
> 
> 
> 
> 
> 
> 
> 
> 
> 
> 
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@ant.apache.org For additional
> commands, e-mail: dev-help@ant.apache.org



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


Re: PR-33: problems

Posted by J Pai <ja...@gmail.com>.
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 <ja...@materne.de> wrote:

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.











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


Re: PR-33: problems

Posted by Nicolas Lalevée <ni...@hibnet.org>.
> Le 29 mai 2017 à 11:35, Jan Matèrne (jhm) <ap...@materne.de> a écrit :
> 
> 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.

It breaks compile time BC but the not binary one, isn’t it ? If it is the case, I have no objection to break it there.

Nicolas


> 
> 
> Jan
> 
>> -----Ursprüngliche Nachricht-----
>> Von: Gintautas Grigelionis [mailto:g.grigelionis@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" <ja...@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.
> 
> 
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@ant.apache.org
> For additional commands, e-mail: dev-help@ant.apache.org
> 


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


AW: PR-33: problems

Posted by "Jan Matèrne (jhm)" <ap...@materne.de>.
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.grigelionis@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" <ja...@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.


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


Re: PR-33: problems

Posted by Gintautas Grigelionis <g....@gmail.com>.
If it's acceptable I'll complement the PR addressing all the points.

Gintas

Den 29 maj 2017 10:13 skrev "Jan Matèrne" <ja...@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.