[
https://issues.apache.org/jira/browse/SANDBOX-481?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14165548#comment-14165548
]
Benedikt Ritter commented on SANDBOX-481:
-----------------------------------------
Hello Luca,
I've look at your patches, here are my comments:
* BaseStage: I don't know the code base well so atm I can not comment whether
it makes sense not to implement process in BaseStage. If process doesn't have
to be implemented in BaseStage, there is no need to define it in BaseStage at
all, since it's already defined in the Stage interface.
* ExtendedBaseStageTest fails because of locale specific number formatting. The
test expects decimal points, but in some locales a comma is used instead. A
better fix would be to find the reason for this and make the test work in all
locales.
* IO Package: This maybe a good idea, also I would expect the Http and Ftp
stuff in a net package... something went wrong with your patch, if only
contained the changes in test_conf.xml and TestResources.properties but not the
moving of the files. Can you recreate this patch please?
* Pipeline: Try to be as general as possible in the interface you present to
the library user. For example it is better to define use Collection<? extends
Stage> or even Iterable<? extends Stage> rather than List<Stage>. Also it's
more convinient to declare methods that take array arguments with varargs.
Best regards,
Benedikt
> [pipeline] improvements
> -----------------------
>
> Key: SANDBOX-481
> URL: https://issues.apache.org/jira/browse/SANDBOX-481
> Project: Commons Sandbox
> Issue Type: Improvement
> Components: Pipeline
> Affects Versions: Nightly Builds
> Reporter: Luca Vercelli
> Attachments: BaseStage.patch, ExtendedBaseStageTest.patch,
> Pipeline.patch, io.patch
>
>
> I find interesting this project, however it appears that there are no commits
> since 2009.
> I performed the following improvements.
> (*) ExtendedBaseStageTest: I had to comment some tests, as they always fail
> (this happened also *before* my modifications!)
> (*) Simplified Pipeline.java. I have added a notion of "default" stage
> factory, and a feed() method.
> (*) BaseStage: process() is now declared abstract. Modified
> RaiseEventStage.java accordingly.
> (*) Created new package org.apache.commons.pipeline.stage.io
> I am planning to create several new stages to be included in this package.
> (*) Moved following files from package org.apache.commons.pipeline.stage to
> org.apache.commons.pipeline.stage.io:
> FileFinderStage, FtpFileDownloadStage, HttpFileDownloadStage
> Modified test files accordingly.
> (*) FileFinder.java: added some new filtering options
> I will attach 4 patches. Patches should be applied /after/ moving files to
> new package stage.io.
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
