----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/2475/#review2708 -----------------------------------------------------------
I've also been meaning to ask you about the whitespace nits. Is there an official policy on them? I try to remove line ending white space any time I see them, I posted a regex to use with eclipse find/replace so I can remove that low hanging fruit from the review before I send it out. Should I not be changing the white space if it already exists? Though you didn't add any, there are some that show up close to the changes you've made. Either way, just trying to understand how this *should* be handled for my future reviews. /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/js/AddJslInfoVariableProcessor.java <https://reviews.apache.org/r/2475/#comment6104> Is there a performance implication here? I actually find the double quotes around a single quote easier on the eyes. /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/oauth2/persistence/OAuth2TokenPersistence.java <https://reviews.apache.org/r/2475/#comment6105> Should we use a StringBuilder here? /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/oauth2/persistence/sample/InMemoryCache.java <https://reviews.apache.org/r/2475/#comment6106> Should we use a StringBuilder here? /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/oauth2/persistence/sample/InMemoryCache.java <https://reviews.apache.org/r/2475/#comment6107> and here? /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/rewrite/ImageAttributeRewriter.java <https://reviews.apache.org/r/2475/#comment6108> Should we use a StringBuilder here? /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/rewrite/RewritePath.java <https://reviews.apache.org/r/2475/#comment6109> StringBuilder? - Dan On 2011-10-20 17:30:46, Paul Lindner wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/2475/ > ----------------------------------------------------------- > > (Updated 2011-10-20 17:30:46) > > > Review request for shindig. > > > Summary > ------- > > Java hygiene for shindig > > * Fix javadoc @param references to renamed variables. > * Remove useless initializers for some variables and restructure some usages. > * Convert several idioms to use isEmpty(). > * Use ternary expressions where they make sense. > * use chars for single-character strings. > * use StringUtils#split or Splitter#on instead of String#split regexes. > * use IOUtils#closeQuietly in finally blocks. > * other stylistic cleanups. > > > Diffs > ----- > > > /trunk/java/common/src/main/java/org/apache/shindig/auth/AnonymousSecurityToken.java > 1186576 > > /trunk/java/common/src/main/java/org/apache/shindig/auth/BlobCrypterSecurityToken.java > 1186576 > > /trunk/java/common/src/main/java/org/apache/shindig/auth/BlobCrypterSecurityTokenCodec.java > 1186576 > > /trunk/java/common/src/main/java/org/apache/shindig/common/PropertiesModule.java > 1186576 > > /trunk/java/common/src/main/java/org/apache/shindig/common/servlet/GuiceServletContextListener.java > 1186576 > > /trunk/java/common/src/main/java/org/apache/shindig/config/ContainerConfigELResolver.java > 1186576 > > /trunk/java/common/src/main/java/org/apache/shindig/expressions/jasper/JasperTypeConverter.java > 1186576 > > /trunk/java/common/src/main/java/org/apache/shindig/protocol/JsonRpcServlet.java > 1186576 > > /trunk/java/common/src/main/java/org/apache/shindig/protocol/conversion/BeanDelegator.java > 1186576 > > /trunk/java/common/src/main/java/org/apache/shindig/protocol/conversion/BeanFilter.java > 1186576 > > /trunk/java/common/src/main/java/org/apache/shindig/protocol/conversion/xstream/ClassFieldMapping.java > 1186576 > > /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/FeedProcessorImpl.java > 1186576 > /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/Gadget.java > 1186576 > > /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/features/FeatureRegistry.java > 1186576 > > /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/features/FeatureResource.java > 1186576 > > /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/http/DefaultRequestPipeline.java > 1186576 > > /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/js/AddJslInfoVariableProcessor.java > 1186576 > > /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/js/AddJslLoadedVariableProcessor.java > 1186576 > > /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/js/AddOnloadFunctionProcessor.java > 1186576 > > /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/js/DeferJsProcessor.java > 1186576 > > /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/js/ExportJsProcessor.java > 1186576 > > /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/js/JsRequest.java > 1186576 > > /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/js/JsResponseBuilder.java > 1186576 > > /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/js/SeparatorCommentingProcessor.java > 1186576 > > /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/oauth/BasicOAuthStore.java > 1186576 > > /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/oauth/OAuthArguments.java > 1186576 > > /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/oauth/OAuthRequest.java > 1186576 > > /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/oauth2/BasicOAuth2Request.java > 1186576 > > /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/oauth2/GadgetOAuth2TokenStore.java > 1186576 > > /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/oauth2/OAuth2Arguments.java > 1186576 > > /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/oauth2/OAuth2Utils.java > 1186576 > > /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/oauth2/handler/BasicAuthenticationHandler.java > 1186576 > > /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/oauth2/handler/CodeAuthorizationResponseHandler.java > 1186576 > > /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/oauth2/handler/CodeGrantTypeHandler.java > 1186576 > > /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/oauth2/handler/TokenAuthorizationResponseHandler.java > 1186576 > > /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/oauth2/persistence/OAuth2Client.java > 1186576 > > /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/oauth2/persistence/OAuth2TokenPersistence.java > 1186576 > > /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/oauth2/persistence/sample/InMemoryCache.java > 1186576 > > /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/oauth2/persistence/sample/JSONOAuth2Persister.java > 1186576 > > /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/oauth2/persistence/sample/OAuth2GadgetBinding.java > 1186576 > > /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/parse/GadgetHtmlParser.java > 1186576 > > /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/parse/nekohtml/NekoSimplifiedHtmlParser.java > 1186576 > > /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/render/DefaultServiceFetcher.java > 1186576 > > /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/render/RenderingGadgetRewriter.java > 1186576 > > /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/render/SanitizingGadgetRewriter.java > 1186576 > > /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/render/SanitizingResponseRewriter.java > 1186576 > > /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/rewrite/ContentRewriterFeature.java > 1186576 > > /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/rewrite/CssResponseRewriter.java > 1186576 > > /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/rewrite/ImageAttributeRewriter.java > 1186576 > > /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/rewrite/ImageResizeRewriter.java > 1186576 > > /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/rewrite/RewritePath.java > 1186576 > > /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/rewrite/ScriptConcatContentRewriter.java > 1186576 > > /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/rewrite/StyleConcatContentRewriter.java > 1186576 > > /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/rewrite/image/JpegImageUtils.java > 1186576 > > /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/rewrite/js/DefaultJsCompiler.java > 1186576 > > /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/servlet/ConcatProxyServlet.java > 1186576 > > /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/servlet/GadgetsHandlerService.java > 1186576 > > /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/servlet/HtmlAccelServlet.java > 1186576 > > /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/servlet/MakeRequestHandler.java > 1186576 > > /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/servlet/ModuleCacheKey.java > 1186576 > > /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/servlet/OAuth2CallbackServlet.java > 1186576 > > /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/servlet/ProxyServlet.java > 1186576 > > /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/servlet/RpcSwfServlet.java > 1186576 > > /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/servlet/ServletUtil.java > 1186576 > > /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/spec/Feature.java > 1186576 > > /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/spec/LocaleSpec.java > 1186576 > > /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/spec/ModulePrefs.java > 1186576 > > /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/templates/TemplateProcessor.java > 1186576 > > /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/templates/tags/CompositeTagRegistry.java > 1186576 > > /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/uri/DefaultConcatUriManager.java > 1186576 > > /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/uri/DefaultIframeUriManager.java > 1186576 > > /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/uri/DefaultJsUriManager.java > 1186576 > > /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/uri/DefaultProxyUriManager.java > 1186576 > > /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/uri/ProxyUriBase.java > 1186576 > > /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/uri/UriUtils.java > 1186576 > > /trunk/java/gadgets/src/main/java16/org/apache/shindig/gadgets/rewrite/js/ClosureJsCompiler.java > 1186576 > > /trunk/java/social-api/src/main/java/org/apache/shindig/social/core/oauth2/OAuth2DataServiceImpl.java > 1186576 > > /trunk/java/social-api/src/main/java/org/apache/shindig/social/core/oauth2/OAuth2NormalizedRequest.java > 1186576 > > /trunk/java/social-api/src/main/java/org/apache/shindig/social/core/oauth2/OAuth2ServiceImpl.java > 1186576 > > /trunk/java/social-api/src/main/java/org/apache/shindig/social/core/oauth2/OAuth2Servlet.java > 1186576 > > /trunk/java/social-api/src/main/java/org/apache/shindig/social/core/oauth2/validators/DefaultResourceRequestValidator.java > 1186576 > > /trunk/java/social-api/src/main/java/org/apache/shindig/social/sample/spi/JsonDbOpensocialService.java > 1186576 > > Diff: https://reviews.apache.org/r/2475/diff > > > Testing > ------- > > mvn build/test, samplecontainer deployment. > > > Thanks, > > Paul > >
