[
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
