Mailing-List: contact issues-help@commons.apache.org; run by ezmlm
Precedence: bulk
Reply-To: issues@commons.apache.org
Date: Mon, 15 Aug 2011 17:31:27 +0000 (UTC)
From: "Matt Benson (JIRA)" <jira@apache.org>
To: issues@commons.apache.org
Message-ID: 
 <1560296905.38805.1313429487301.JavaMail.tomcat@hel.zones.apache.org>
In-Reply-To: 
 <150634813.38651.1313426735200.JavaMail.tomcat@hel.zones.apache.org>
Subject: [jira] [Commented] (CHAIN-53) Global Update of Chain - Generics,
 JDK 1.5, Update Dependency Versions
MIME-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Transfer-Encoding: 7bit


    [ https://issues.apache.org/jira/browse/CHAIN-53?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13085184#comment-13085184 ] 

Matt Benson commented on CHAIN-53:
----------------------------------

Hello again Elijah,
  I have looked over the diff; here are some comments:

* diffs should be attached/uploaded in JIRA, with the grant/feather radio button checked indicating your intent that the patch be licensed to the ASF (I know you sent the ICLA, but a. it wouldn't have been processed yet, and b. just humor us ;)  )
* I don't see anything in the Faces-related changes to warrant upgrading to JSF 2.x.  MyFaces in particular makes every attempt to continue to support JSF 1.x versions, so in the spirit of good inter-ASF cooperation, we should probably just leave the API levels of the JSF dependency wherever they stood previously.
* At Commons we often repackage components when their APIs change incompatibly.  The changes you have submitted are overwhelmingly backward-compatible once type erasure has been taken into account.  What I particularly notice as being backward-incompatible are the {{Map}} implementations.  Since most of these have gone from raw {{Map}} to {{Map<String, ?>}} their {{put()}} methods now have different signatures.  In all cases except for {{oac.chain.web.servlet.ServletApplicationScopeMap}} these keys are required to be {{String}} instances at runtime anyway, so there is quite a minimal chance that code currently using these wouldn't recompile against these binaries.  In the last case, {{null}} keys are rejected and other objects are converted to {{String}} if necessary.  Once again, it seems rather unlikely that existing code would be utilizing this conversion code path.

The {{Map}} concerns are the only potential point of contention I see with regard to backward compatibility.  It would seem to me that [chain] is likely to sit rather high in the architecture of a given application, with little chance of multiple consumers competing at runtime.  For this reason my personal opinion is that the incompatibilities introduced in the process of generifying the provided {{Map}} implementations are small enough to consider the component backward-compatible _enough_ and accept this patch directly onto [chain]'s trunk.  I point the situation out here, however, in case other members of the community, particularly those with actual _experience_ with [chain], have conflicting opinions.

Thanks for your interest!

> Global Update of Chain - Generics, JDK 1.5, Update Dependency Versions
> ----------------------------------------------------------------------
>
>                 Key: CHAIN-53
>                 URL: https://issues.apache.org/jira/browse/CHAIN-53
>             Project: Commons Chain
>          Issue Type: Improvement
>            Reporter: Elijah Zupancic
>              Labels: newbie, patch
>
> As posted in the mailing list, I've done this work outside of an offical branch.
> Here is the source:
> http://elijah.zupancic.name/projects/commons-chain-v2-proof-of-concept.tar.gz
> And here is a diff:
> http://elijah.zupancic.name/projects/uber-diff
> In this patch:
> * Global upgrade to the JDK 1.5
> * Added @Override annotations
> * Upgraded to the Servlet 2.5 API
> * Upgraded to the Faces 2.1 API
> * Upgraded to the Portlet 2.0 API
> * Upgraded the Maven Parent POM version
> * Added generics support to Command so that Command's API looks like:
> public interface Command<T extends Context> {
> ...
>    boolean execute(T context) throws Exception;
> }
> I'm very much new to the ASF and I was advised to file a bug in order to get the process started for these changes to be integrated.

--
This message is automatically generated by JIRA.
For more information on JIRA, see: http://www.atlassian.com/software/jira