----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/7471/#review12633 -----------------------------------------------------------
Great job! I tested it and it works fine - just minor comments. ./src/org/waveprotocol/box/server/persistence/file/FileAttachmentStore.java <https://reviews.apache.org/r/7471/#comment26965> proto.getPB() -> Can we rename getPB() method into something more meaningful? ./src/org/waveprotocol/box/server/rpc/AttachmentServlet.java <https://reviews.apache.org/r/7471/#comment26966> Can we extract this line into a method with corresponding name instead of adding a comment? ./src/org/waveprotocol/wave/client/doodad/attachment/AttachmentManagerImpl.java <https://reviews.apache.org/r/7471/#comment26967> Empty line ./src/org/waveprotocol/wave/client/doodad/attachment/AttachmentManagerImpl.java <https://reviews.apache.org/r/7471/#comment26968> Please move the method declaration below fields declaration ./src/org/waveprotocol/wave/media/model/Attachment.java <https://reviews.apache.org/r/7471/#comment26970> Remove the Google header ./src/org/waveprotocol/wave/media/model/Attachment.java <https://reviews.apache.org/r/7471/#comment26969> Empty line - Yuri Zelikov On Oct. 17, 2012, 11:55 a.m., Andrew Kaplanov wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/7471/ > ----------------------------------------------------------- > > (Updated Oct. 17, 2012, 11:55 a.m.) > > > Review request for wave and Yuri Zelikov. > > > Description > ------- > > -- Storage > > As in original version, attachments stored in the directory, defined in the > parameter attachment_store_directory of server.config. > But now all attachments with thumbnails and metadata stored in single > directory. > If you have attachments in your instance of Wiab, move files from > subdirectories in attachment_store_directory up and remove subdirectories. > > -- Thumbnails > > Image attachment shown in the wave as the reduced picture. > Not image attachment shown as icon, representing type of this attachment. > In this case icon is taken from the directory, defined in parameter > thumbnail_patterns_directory of server.config. > Icon must be in PNG format, and named as MIME type with replacing '/' to '_'. > For example thumbnail file for ZIP format (MIME type application/zip) must be > named application_zip. > > > Diffs > ----- > > ./build-proto.xml 1393974 > ./build.xml 1393974 > ./proto_src/org/waveprotocol/box/attachment/AttachmentProto.java > PRE-CREATION > ./server-config.xml 1393974 > ./src/org/waveprotocol/box/attachment/Attachment.gwt.xml PRE-CREATION > ./src/org/waveprotocol/box/attachment/attachment.proto PRE-CREATION > ./src/org/waveprotocol/box/server/CoreSettings.java 1393974 > ./src/org/waveprotocol/box/server/ServerMain.java 1393974 > ./src/org/waveprotocol/box/server/attachment/AttachmentService.java > PRE-CREATION > ./src/org/waveprotocol/box/server/persistence/AttachmentStore.java 1393974 > ./src/org/waveprotocol/box/server/persistence/AttachmentUtil.java 1393974 > ./src/org/waveprotocol/box/server/persistence/file/FileAttachmentStore.java > 1393974 > ./src/org/waveprotocol/box/server/persistence/mongodb/MongoDbStore.java > 1393974 > ./src/org/waveprotocol/box/server/rpc/AttachmentInfoServlet.java > PRE-CREATION > ./src/org/waveprotocol/box/server/rpc/AttachmentServlet.java 1393974 > ./src/org/waveprotocol/box/server/rpc/ProtoSerializer.java 1393974 > ./src/org/waveprotocol/box/webclient/WebClient.gwt.xml 1393974 > ./src/org/waveprotocol/wave/client/StageTwo.java 1393974 > ./src/org/waveprotocol/wave/client/doodad/attachment/AttachmentImpl.java > PRE-CREATION > > ./src/org/waveprotocol/wave/client/doodad/attachment/AttachmentManagerImpl.java > PRE-CREATION > > ./src/org/waveprotocol/wave/client/doodad/attachment/ImageThumbnailAttachmentHandler.java > 1393974 > > ./src/org/waveprotocol/wave/client/doodad/attachment/ImageThumbnailNodeEventHandler.java > 1393974 > > ./src/org/waveprotocol/wave/client/doodad/attachment/SimpleAttachmentManager.java > 1393974 > > ./src/org/waveprotocol/wave/client/doodad/attachment/render/ImageThumbnailRenderer.java > 1393974 > > ./src/org/waveprotocol/wave/client/doodad/attachment/render/ImageThumbnailWidget.java > 1393974 > > ./src/org/waveprotocol/wave/client/doodad/attachment/render/ImageThumbnailWrapper.java > 1393974 > ./src/org/waveprotocol/wave/client/doodad/attachment/render/Thumbnail.css > 1393974 > > ./src/org/waveprotocol/wave/client/doodad/attachment/testing/FakeAttachment.java > 1393974 > > ./src/org/waveprotocol/wave/client/doodad/attachment/testing/FakeAttachmentsManager.java > 1393974 > ./src/org/waveprotocol/wave/client/wavepanel/impl/toolbar/EditToolbar.java > 1393974 > ./src/org/waveprotocol/wave/media/model/Attachment.java PRE-CREATION > ./src/org/waveprotocol/wave/media/model/AttachmentDocumentWrapper.java > 1393974 > ./src/org/waveprotocol/wave/media/model/AttachmentV3.java 1393974 > ./src/org/waveprotocol/wave/media/model/ClientAttachment.java 1393974 > ./src/org/waveprotocol/wave/media/model/MutableClientAttachment.java > 1393974 > ./test/org/waveprotocol/box/server/persistence/AttachmentStoreTestBase.java > 1393974 > ./test/org/waveprotocol/wave/media/model/AttachmentDocumentWrapperTest.java > 1393974 > > Diff: https://reviews.apache.org/r/7471/diff/ > > > Testing > ------- > > > Thanks, > > Andrew Kaplanov > >
