Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Adding support for Download builder #791

Merged
merged 10 commits into from
Sep 12, 2014
Merged

Conversation

garcia-jj
Copy link
Member

Closes #789. As discussed in dev mailing list and under issue #789.

I don't know if we use all builder subclasses as inner classes or external classes. Ideas?

After all comments, I can write some docs and improve tests for bytearray and inputstream.

@renanigt
Copy link
Contributor

Cool. 👍

@garcia-jj
Copy link
Member Author

@guilhermesilveira As you suggested :). Thank you.

* .build();
* </code>
*/
public static InputStreamDownloadBuilder of(InputStream input) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't like this kind of javadoc. This is hard to maintain and kinda heavy.
I think a succinct description would be enough here (and keep our docs
symmetry)... we can share this full explained code on VRaptor's site docs.

public class FileDownloadTest {

@Rule
public ExpectedException thrown = ExpectedException.none();
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This rule was a @csokol suggestion, and I think we can use from now, because we can capture messages and more. Using @Test(expected) we can't take full controll of exception thrown.

@lucascs
Copy link
Member

lucascs commented Sep 11, 2014

🐑

garcia-jj added a commit that referenced this pull request Sep 12, 2014
Adding support for Download builder
@garcia-jj garcia-jj merged commit f6d9de5 into master Sep 12, 2014
@garcia-jj garcia-jj deleted the ot-gh789-downloadbuilder branch September 12, 2014 18:41
}

public static abstract class AbstractDownloadBuilder<T> {
protected String fileName;
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Devs, what you think to reducing visibility of this class, since only builder can see this? There is no reason to expose this class to the world.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Create a builder to return Download instances
5 participants