axis-java-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Afkham Azeez <afk...@gmail.com>
Subject Re: Please revert API changes done as per AXIS2-4465
Date Fri, 21 Aug 2009 06:38:41 GMT
Andreas,
I do appreciate your interest in the Apache WS other projects & also
welcome your contributions. I'm sure that your contributions have
improved the quality of our stuff. My intention was not to offend you.
There are many things that are pretty ugly in Axis2. Many of the
interfaces & public methods are badly named. However, Axis2 has
reached a certain level of maturity now, hence we cannot fix these
things and people have learned to live with it. Ideally, we should
have done these type of changes before Axis2 1.0, but we cannot change
the past now :(

With respect to my API change to clustering;

1. The clustering implementation was developed primarily by me
2. I did the implementation in Axis2 as well as  Synapse
3. The clustering implementation went through 3 major iterations; now
I think we've got it right to a major extent
4. There is only a single clustering implementation; the Tribes based one
5. The clustering stuff is fairly new, compared to the other parts of Axis2
6. When I made the changes to the API, I made sure that the
corresponding changes were done on Synapse too. At that time, Synapse
depended on Axis2 trunk. It was too late to get the API changes into
Axis2 1.5
7. So I think my API changes were appropriate since it was done early
in the development cycle of the cluster module. As time passes by, it
becomes much more difficult to introduce such changes.

Thanks
Azeez

On Thu, Aug 20, 2009 at 6:50 PM, Andreas
Veithen<andreas.veithen@gmail.com> wrote:
> All,
>
> Just to clarify some things:
>
> 1) I explained the purpose of the fix in AXIS2-4465. I believe that
> the description gives enough information about the why and the how of
> the change. If not, point to the parts that are not clear. Please also
> have a look at the linked issues to see the kind of problems the code
> had before the fix. While doing the changes I inadvertently removed a
> public method. As explained above, this can be easily fixed, and I
> will do so.
>
> 2) I think there is no need to start yelling around about "merrily
> chang[ing] APIs", "things are going to become a big mess" and "engage
> the community before making such drastic changes". I did lots of fixes
> in Axiom and Axis2 since Synapse trunk switched from Axis2 SNAPSHOT to
> 1.5, and when Hiranya switched back to SNAPSHOT there were no
> particular issues related to them, except for the issue discussed
> here. I personally take the suggestion that my changes make of Axis2 a
> big mess as an offense. If that is what some people think, I can stop
> immediately to work on Axis2 and Axiom. With respect to engaging the
> community, as mentioned above, the description of AXIS2-4465 provides
> enough information about the reasons for the change and as Dims
> pointed out, we have a commit-and-review policy. Since the change is
> focused, not massive, can easily be reverted and contains an
> appropriate level of Javadoc, I think that the community has
> everything that is needed to review it.
>
> Finally, I could ask in reply how all these principles applied to the
> change in r744900 [2], which incidentally is the second change that
> caused problems when switching Axis2 versions on the Synapse trunk and
> which did not address any bug at all.
>
> 3) The discussion in AXIS2-4465 pointed to the problem that even by
> preserving the public API of AxisServlet, we will probably not be able
> to avoid breaking subclasses. This is caused by the fact that
> AxisServlet doesn't have an extensible design. If somebody has a
> brilliant idea how to get around this problem (other than reverting
> the fix and reopening the 6 issues it is supposed to solve), please
> speak out.
>
> Andreas
>
> [1] http://markmail.org/message/62dixjx3qrqry3yr
> [2] http://svn.apache.org/viewvc?view=rev&revision=744900
>
> On Thu, Aug 20, 2009 at 18:08, Senaka Fernando<senaka@wso2.com> wrote:
>> Hi Andreas,
>>
>> Just wondering what you are trying to achieve here. Is this related to auto
>> detection of ports as Hiranya pointed out? While I appreciate the effort
>> you've put into doing something worthwhile, I believe that getting rid of a
>> public method in a class is not the correct thing to do. I believe that what
>> you have done here is the addition of a new portion of code. Can we make the
>> new portion of code optional? And leave the existing logic as it was? Also,
>> are you planning further changes to this class? if so, it would perhaps be
>> better to figure out a more elaborate solution, which safeguards both the
>> existing level of extensibility of this class and also its public API.
>>
>> Thanks,
>> Senaka
>>
>> On Thu, Aug 20, 2009 at 6:14 PM, Hiranya Jayathilaka <hiranya911@gmail.com>
>> wrote:
>>>
>>> Hi Andreas,
>>>
>>> By looking at the code I got the impression that HTTP transport receivers
>>> should extend the AxisServletListener class for your logic of port auto
>>> detection to work. Is that correct? What happens if the transport receivers
>>> used do not extend this class? All request handler methods call the
>>> preprocessRequest method which in turns run port auto detection. If the
>>> transport receivers do not extend AxisServlerListener how is that handled?
>>>
>>> Thanks,
>>> Hiranya
>>>
>>>
>>> On Thu, Aug 20, 2009 at 6:05 PM, Andreas Veithen
>>> <andreas.veithen@gmail.com> wrote:
>>>>
>>>> Afkham,
>>>>
>>>> The only change I see in the public APIs is the disappearance of the
>>>> initContextRoot method. We can easily fix this be restoring the
>>>> original initContextRoot method and let the preprocessRequest method
>>>> call initContextRoot. Do you see any other things to change?
>>>>
>>>> Andreas
>>>>
>>>> On Thu, Aug 20, 2009 at 13:45, Afkham Azeez<afkham@gmail.com> wrote:
>>>> > Yes Dims. However, if everybody continues to merrily change APIs,
>>>> > making public methods private & so on, things are going to become
a
>>>> > big mess. Axis2 provides public APIs, and those may be having
>>>> > problems, but still they are public APIs. This is why you have to be
>>>> > very careful when defining APIs; if you get them wrong, you may have
>>>> > to live with it for a long time.
>>>> >
>>>> > Azeez
>>>> >
>>>> > On Thu, Aug 20, 2009 at 11:38 AM, Davanum Srinivas<davanum@gmail.com>
>>>> > wrote:
>>>> >> Azeez,
>>>> >>
>>>> >> We are still following, commit-then-review right?
>>>> >>
>>>> >> thanks,
>>>> >> dims
>>>> >>
>>>> >> On 08/20/2009 07:33 AM, Afkham Azeez wrote:
>>>> >>>
>>>> >>> Hi Andreas,
>>>> >>> The changes you've done to the APIs as per
>>>> >>> https://issues.apache.org/jira/browse/AXIS2-4465 badly breaks
some of
>>>> >>> the projects that depend on Axis2. Please revert this, and please
>>>> >>> engage the community before making such drastic changes in the
>>>> >>> future.
>>>> >>>
>>>> >>
>>>> >
>>>> >
>>>> >
>>>> > --
>>>> > Thanks
>>>> > Afkham Azeez
>>>> >
>>>> > Blog: http://afkham.org
>>>> > Developer Portal: http://www.wso2.org
>>>> > WSAS Blog: http://wso2wsas.blogspot.com
>>>> > Company: http://wso2.com
>>>> > GPG Fingerprint: 643F C2AF EB78 F886 40C9  B2A2 4AE2 C887 665E 0760
>>>> >
>>>
>>>
>>>
>>> --
>>> Hiranya Jayathilaka
>>> Software Engineer;
>>> WSO2 Inc.;  http://wso2.org
>>> E-mail: hiranya@wso2.com;  Mobile: +94 77 633 3491
>>> Blog: http://techfeast-hiranya.blogspot.com
>>
>>
>



-- 
Thanks
Afkham Azeez

Blog: http://afkham.org
Developer Portal: http://www.wso2.org
WSAS Blog: http://wso2wsas.blogspot.com
Company: http://wso2.com
GPG Fingerprint: 643F C2AF EB78 F886 40C9  B2A2 4AE2 C887 665E 0760

Mime
View raw message