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

Reusable file object #79

Merged
merged 4 commits into from
May 31, 2018
Merged

Reusable file object #79

merged 4 commits into from
May 31, 2018

Conversation

Shudrum
Copy link
Contributor

@Shudrum Shudrum commented May 22, 2018

Hello there!

First of all: thank you for this project! Life & time saver.

I am using this middleware on several projects, and I have, on all of those project, the same problem:

How can I correctly test my file uploading system?

The problem is:
I do not only upload files, but I also do some business transformation on them. As I try to keep everything as simple as possible: I just use your file object without decorator or anything else.

So, for the tests… it need many boilerplates or… a decorator…

This is why this pull request:

I would love to have a real FileObject / File or just a factory. I choose the factory option to keep this project also simple.

The steps are split on several commits. If you agree on the principe, I would like to talk about the last one: 64ff2b5

I've exposed the fileFactory, so people who want to use it can simply do:

const fileFactory = require('express-fileupload').fileFactory;

If you just want to replace it by:

const fileFactory = require('express-fileupload/lib/fileFactory');

Just ask.

To finish, as it is quite like a low level functionality, I don't think it should be documented, people who want to test it can find it easily by looking at this repository.

Thank you!

@coveralls
Copy link

coveralls commented May 22, 2018

Coverage Status

Coverage increased (+2.2%) to 98.0% when pulling 87d3d5c on Shudrum:reusable-file-object into 9314d38 on richardgirges:master.

@r3wt
Copy link
Contributor

r3wt commented May 22, 2018

this is awesome. great work @Shudrum

@Shudrum
Copy link
Contributor Author

Shudrum commented May 22, 2018

Thank you ☺️

@richardgirges
Copy link
Owner

NICE! Give me a day or two to digest this, I may have some questions. But I agree with the direction and need for this! It'll help me out too for testing.

Copy link
Owner

@richardgirges richardgirges left a comment

Choose a reason for hiding this comment

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

I'm doing a small revamp of the project. I'm going to merge this is in and make my changes on top of your changes (adding new options, changing some lint rules, etc). Thanks so much for taking the time to push this!

@richardgirges richardgirges merged commit 536321b into richardgirges:master May 31, 2018
@Shudrum
Copy link
Contributor Author

Shudrum commented May 31, 2018

You're welcome!

@Shudrum Shudrum deleted the reusable-file-object branch May 31, 2018 22:11
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.

4 participants