Sorry, but I must vote -1 on this.

Most of the issues I raised earlier about the ExtensionsFilter refactoring have still not been addressed.

One change has been made: the ExtensionsFilter now does actually do the work, and the FacesContextFactory does not wrap the object in that case.

But the rest of the concerns I had still exist. In particular, there is a large amount of new code that exists to parse the web.xml file looking for config properties - and I don't see why any of this code should exist. I had thought that an earlier email by you (Leonardo) meant that you agreed and would change this to just looking for servlet init parameters via the normal APIs.

In particular, I think we should delete the ExtensionsFilterConfig class completely.

Minor points:
* Why is new class AbstractAttributeMap in the "servlet" package?
* Should AbstractAttributeMap be named "_AbstractAttributeMap" instead, ie is it part of tomahawk's "stable public api" or just an internal implementation detail?
* New classes should have @since annotations
* etc

Regards,
Simon

Reply via email to