incubator-wave-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Yuri Zelikov" <vega...@gmail.com>
Subject Re: Review Request: Attachments.
Date Sun, 21 Oct 2012 16:36:04 GMT

-----------------------------------------------------------
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
> 
>


Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message