Hi, Here are a few minor things I noticed about the Orchestra code:
===================== MockFrameworkAdapter is checked in twice * core/src/test/java/org/apache/myfaces/orchestra/frameworkAdapter * core/src/main/java/org/apache/myfaces/orchestra/frameworkAdapter This causes Eclipse to complain (as it should). Is there a reason to have two copies, or is this just a mistake? AIUI, this class is intended to be shipped to users so the second occurrence is the one to keep? ===================== There are a number of classes that are Serializable but do not declare a serialVersionUID value. Serializable classes really should, so that later changes which don't break serializable compatibility but do change the class checksum can be correctly handled as compatible. Normally, I like starting serialVersionUID at 1 and incrementing from there for each incompatible change. However when a class is already in use in production without an explicit serialVersionUID then there is a valid argument for starting from the current default serialVersionUID (the class checksum). I don't think orchestra is widely enough used yet to worry about compatibility, so how about we add private static final long serialVersionUID = 1; to all Serializable classes? ===================== In ConversationManager, constant CONVERSATION_CONTEXT_PARAM is public. The only user (and the only sensible user) is class ConversationRequestParameterProvider in the same package. So could this be made package-scope to avoid exposing this constant in the user API? Cheers, Simon
