Skip to content

[MRESOLVER-63] FileTransformer#transformData(File) should also throw TransformException#23

Merged
rfscholte merged 1 commit intoapache:masterfrom
mcimbora:MRESOLVER-63
Nov 16, 2018
Merged

[MRESOLVER-63] FileTransformer#transformData(File) should also throw TransformException#23
rfscholte merged 1 commit intoapache:masterfrom
mcimbora:MRESOLVER-63

Conversation

@mcimbora
Copy link
Copy Markdown
Contributor

Up-for-grabs #DevoxxBE

Copy link
Copy Markdown
Contributor

@rfscholte rfscholte left a comment

Choose a reason for hiding this comment

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

Thanks for picking this up so fast! Have a look at the comments and let's see if this can be improved soon.

* Thrown when file transformation failed.
*/
public class FileTransformationException
extends RepositoryException
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This is not really a repository exception. Consider the following usecase: we want to transform the pom.xml during upload with xslt. When trying to generate the XMLParser you must catch the SaxException. This one should be wrapped and thrown by this method.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Good point, thanks for the explaination and prompt review. The exception now inherits directly from Exception.

*
* @param file The file which transformation failed, may be {@code null}.
*/
public FileTransformationException( File file )
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

There's not always a file involved yet, no need to use this.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Removed.

* @return the transformed data
*/
InputStream transformData( File file ) throws IOException;
InputStream transformData( File file ) throws FileTransformationException;
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Both IOException and TransformException must be thrown. Better not make it too explicit, we might do other transformations as well in the future, not only filebased.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Included TransformException in the throws clause.

@rfscholte
Copy link
Copy Markdown
Contributor

If you want you can add yourself as a contributor in the pomfile as well.

@mcimbora
Copy link
Copy Markdown
Contributor Author

@rfscholte PR updated, added contributor section to pom.xml.

@rfscholte rfscholte merged commit 2e57238 into apache:master Nov 16, 2018
@jira-importer
Copy link
Copy Markdown

Resolve #844

1 similar comment
@jira-importer
Copy link
Copy Markdown

Resolve #844

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants