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

refactor to inherit from collection #47

Merged
merged 7 commits into from
Jan 18, 2016
Merged

refactor to inherit from collection #47

merged 7 commits into from
Jan 18, 2016

Conversation

EvilDrW
Copy link
Contributor

@EvilDrW EvilDrW commented Dec 21, 2015

in my opinion, it is better for the fileupload resource to inherit from collection, not resource. doing so gives us the advantage of maintaining a consistent interface with deployd (queries, script domains, etc. are identical to what deployd supplies).

in addition, it makes the code much shorter and most importantly gives us access to the dashboard to show all of the files.

as an aside, i sacrificed the option to not have unique filenames. to me it was problematic because of the possibility of clobbering someone else's files.

EvilDrW and others added 3 commits November 17, 2015 10:03
add subdir to the stored object so when you get it, you know where to actually retrieve the file from
makes all of the standard deployd queries work, makes the script domains
consistent with deployd, makes the dashboard work as expected
@EvilDrW
Copy link
Contributor Author

EvilDrW commented Dec 21, 2015

additionally, this addresses #10 and #8 (the latter because it enforces always using unique filenames)

@NicolasRitouet
Copy link
Owner

Looks good. 👍
Give me a few days to review this.
(and can you fix the conflicts?)

EvilDrW added 4 commits December 28, 2015 20:05
@EvilDrW
Copy link
Contributor Author

EvilDrW commented Dec 29, 2015

ok, i got this done. i might need to do a PR for the demo repo as well to support the changes.

@NicolasRitouet NicolasRitouet self-assigned this Dec 29, 2015
@NicolasRitouet
Copy link
Owner

@EvilDrW
Sorry for the delay, I'll try to review your PR in the coming days.

@israelroldan
Copy link

I have been testing this fork and it seems like a great step forward! PUT support is definitely missed, especially if you just want to update a non-file-related field, but there are ways to work around this of course. 👍

@NicolasRitouet
Copy link
Owner

@israelroldan thanks for the feedback, I'll take care of this (I also have to publish a new release of deployd :/ )

NicolasRitouet added a commit that referenced this pull request Jan 18, 2016
refactor to inherit from collection
@NicolasRitouet NicolasRitouet merged commit b81a660 into NicolasRitouet:master Jan 18, 2016
@NicolasRitouet
Copy link
Owner

Thanks a lot @EvilDrW for your contribution!

@NicolasRitouet
Copy link
Owner

@EvilDrW
this change has been released as an 1.0.0-alpha version and is available under the next tag:
npm install dpd-fileupload@next --save

Let me know if all works fine, and I'll make a final release

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