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

Full base Implementation of node-uploads #2

merged 34 commits into from Apr 17, 2019


None yet
3 participants
Copy link

bericp1 commented Apr 12, 2019

This PR is a merge of master (what's released onto npm right now) into a temporary branch upstream-base which represents essentially the starting point for this PR.

As such, this PR contains the entire functional implementation of node-uploads up to this point (v1.0.2).

bericp1 added some commits Apr 4, 2019

feat(*): implement the base logic for uploading files and handling disks
Still need to implement delete and transfer and write some tests for the repository functionality
before this is ready to ship.
revert(ci): undo prepack for git install
Due to yarnpkg/yarn#5047, prepack doesnt get triggered with yarn so we cant do direct git installs
in a consistent way unfortunately.
fix(*): write tests for service and fix revealed bugs

-   Add `MemoryRepository` (basically an in-memory db) to support tests
-   Test the entire Uploads public API surface area
-   Fix revealed bugs from tests
-   Actually delete on update (to mimc file move)
-   Install memfs in order to use a memory disk for testing
-   Use process.hrtime for generation of filenames to get unique timestamps even in the same tick
docs(*): detailed usage docs
Also, reorganize the transfer arguments to make more sense

bericp1 added some commits Apr 12, 2019

fix(types): add types for tmp
We do not explicitly depend on tmp however we use types from the @carimus/node-disks package which does use types from tmp so we need to know about them.

This comment has been minimized.

Copy link
Member Author

bericp1 commented Apr 12, 2019

@bericp1 bericp1 requested review from KristenKatona and udaycarimus Apr 12, 2019

Copy link

KristenKatona left a comment

This a LOT to look at but it seems like you have good coverage with unit tests so I'll assume they all passed and that we're going to test this a lot when its in our project.

Just one comment below about a TODO I saw that we should cover with a task somewhere so it doesn't get lost/forgotten!

* without regeneration, it will still use the old pathPrefix and not any new one if its changed in the config
* (since the pathPrefix is applied during the generation of the filename).
* TODO Once @carimus/node-disks supports the copy operation, use that when oldDisk === newDisk

This comment has been minimized.

Copy link

KristenKatona Apr 16, 2019

Does a task need to be generated somewhere to do this or is this already noted in a task for when this will occur? (i.e. do we have a task to update @carimus/node-disks to support the copy operation and/or to make this update one it does)

This comment has been minimized.

Copy link

bericp1 Apr 16, 2019

Author Member

We don't have a task because there's nowhere to track tasks for these common projects yet :/ Once we set up a JIRA project for these things, I'll do the honors of digging through and finding all of these TODOs and creating tasks for them.

@udaycarimus udaycarimus merged commit b5e3b59 into upstream-base Apr 17, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.