Refactor ServletFacet --------------------- Key: FORGE-1137 URL: https://issues.jboss.org/browse/FORGE-1137 Project: Forge Issue Type: Enhancement Components: Java EE APIs Affects Versions: 2.0.0.Alpha11 Reporter: George Gastaldi {quote} <gastaldi> in Forge 2.x, should the ServletFacetImpl.isInstalled() check for web.xml existence to assert that it is installed ? <gastaldi> I think it should only check if the required deps are met <gastaldi> and @RequireFacet(WebResourcesFacet.class) perhaps <lincolnthree> gastaldi: why this change? (leading question) <gastaldi> FORGE-1135 <jbossbot> jira [FORGE-1135] ServletFacetImpl overrides isInstalled of BaseJavaEEFacet but doesnt call super.isInstalled [Closed (Done) Bug, Major, Thomas Frühbeck] https://issues.jboss.org/browse/FORGE-1135 <gastaldi> if we think about it, checking for the web.xml presence doesn't make much sense in it <lincolnthree> why is that? <gastaldi> suppose you're writing a java library that depends on Servlet API <vineetreynolds> Servlet 3.0 <gastaldi> yes <gastaldi> Why would you need web.xml in it? <lincolnthree> im playing devil's advocate here <vineetreynolds> Even web-framgment.xml changes the equation <lincolnthree> i'd say that we should have 2 facets then, or 3 facets <vineetreynolds> It could be a jar with a web-fragment.xml <lincolnthree> ServletFacet25 <lincolnthree> ServletFacet30 <vineetreynolds> Yes, like SW <vineetreynolds> Activate based on version value in the XML <vineetreynolds> version="2.5", version="3.0" <lincolnthree> ServletFragmentFacet30 <gastaldi> I think we should move the org.jboss.forge.addon.javaee.facets.ServletFacet.getConfig() to another facet <gastaldi> WebResourcesFacet perhaps <lincolnthree> gastaldi: WebResourceFacet is even less tied to servlet than servlet it <lincolnthree> is <lincolnthree> that just adds src/web/resources <lincolnthree> err <lincolnthree> src/main/webapp <vineetreynolds> Right <gastaldi> sure, just saying that maybe it doesn't make much sense to be in ServletFacet <vineetreynolds> Do we even need a ServletFacet ? <lincolnthree> gastaldi: I don't see why not <vineetreynolds> nvm <vineetreynolds> We need a facet for the dependency <lincolnthree> vineetreynolds: yes <vineetreynolds> ServletAPIFacet <gastaldi> yeah, something like that <lincolnthree> ServletApiFacet30 <gastaldi> like Faces does <vineetreynolds> I dont think we should tie to the web.xml file <vineetreynolds> unless we need to <lincolnthree> vineetreynolds: if there is a web.xml file, it should be respected <lincolnthree> if there is not, it should not be required for 3.0 <lincolnthree> but for 2.5 it should be <vineetreynolds> Well, the dependency is more important imho <gastaldi> lincolnthree, unless the PackagingType != WAR <gastaldi> if it's a library, web.xml is never required in the current project <lincolnthree> gastaldi: sure <gastaldi> hence, should use named web-fragment <gastaldi> s/use/be <lincolnthree> gastaldi: again… only in 3.0 <gastaldi> yea <gastaldi> in 2.5 should not rely on any descriptor if PackagingType is not WAR <lincolnthree> correct {quote}