[jbossseam-issues] [JBoss JIRA] Commented: (JBSEAM-2775) package-info.java should be optional:"convention over configuration"

[ http://jira.jboss.com/jira/browse/JBSEAM-2775?page=comments#action_12409385 ] Eric Jung commented on JBSEAM-2775: ----------------------------------- Hi Norman, Nice work. Here are my comments: 0. You didn't take my updates to the seampay example, demonstrating how this new feature works. Did you miss those changes or was that intentional? I think it's a good idea to showcase this in an example if we want people to use it :) I probably should have added a comment to components.xml explaining that the "pay" namespace is automatically resolved, instead of just changing the namespace. 1. You didn't update my class-level javadoc in NamespacePackageResolver.java, so it's out-of-date with what you checked into the trunk. For example, it still references the seam: namespace instead of java:. Let me know if you'd like me to submit a patch with just javadoc updates. I'd be happy to do it. 2. If you don't like the support for hierarchical URIs such as http://www.company.com/dept/product, I think you should remove it entirely. Since this is a feature upgrade to seam, people will be happy with the new feature even if it just supports opaque URIs. In 2008, java developers with more than just a basic understanding of XML comprehend that URI != URL, and won't be confused or have the need to specify only hierarchical URIs as the namespace. Furthermore, dropping support for hierarchical URLs will reduce the code significantly. 3. You removed a few things from my code which have me wondering why: a. You removed isValidJavaPackageName() but kept makeSafeForJava(). Why not drop both or keep both? Partial support for package name checking and fixing seems odd to me: why not go all the way or drop the whole idea? If you drop the idea, the class-level javadoc needs updating. b. For hierarchical URI parsing, you removed the part which ignores any leading www subdomain. If you're going to keep support for hierarchical URIs, I think this is important to keep. It makes the parsing a little smarter, and you know people are going to use their company's URL as their namespace.... corporate URLs generally begin with www but, by convention, no one includes www in their package structures. 4. Finally, I think makeSafeForJava() should implement all 3 bullet points at http://java.sun.com/docs/books/jls/third_edition/html/packages.html#7.7, not just the first bullet point. Sorry, I forget to implement the 2nd two. I'd be happy to submit a patch for any/all of the above changes against trunk. Let me know how I can help. Thanks, Eric -- This message is automatically generated by JIRA. - If you think it was sent incorrectly contact one of the administrators: http://jira.jboss.com/jira/secure/Administrators.jspa - For more information on JIRA, see: http://www.atlassian.com/software/jira