Skip to content
This repository has been archived by the owner on Apr 16, 2021. It is now read-only.

Support downloading skylinks with paths and query params #76

Merged
merged 17 commits into from
Nov 18, 2020

Conversation

mrcnski
Copy link
Contributor

@mrcnski mrcnski commented Sep 9, 2020

src/download.js Outdated Show resolved Hide resolved
src/utils.js Outdated Show resolved Hide resolved
@mrcnski
Copy link
Contributor Author

mrcnski commented Oct 19, 2020

@kwypchlo Pls review

src/utils.test.ts Outdated Show resolved Hide resolved
@mrcnski mrcnski requested a review from kwypchlo October 20, 2020 14:43
src/utils.ts Outdated Show resolved Hide resolved
src/download.ts Outdated Show resolved Hide resolved
src/download.ts Show resolved Hide resolved
src/upload.test.ts Outdated Show resolved Hide resolved
src/utils.test.ts Show resolved Hide resolved
src/utils.ts Outdated Show resolved Hide resolved
src/utils.test.ts Outdated Show resolved Hide resolved
src/utils.ts Outdated Show resolved Hide resolved
src/download.test.ts Outdated Show resolved Hide resolved
src/download.test.ts Outdated Show resolved Hide resolved
@mrcnski mrcnski requested a review from kwypchlo November 13, 2020 16:18
@mrcnski
Copy link
Contributor Author

mrcnski commented Nov 13, 2020

Ready for another review @kwypchlo. I still need to add tests for getting the subdomain skylink from getSkylinkUrl, I'll do that if everything else looks good.

Copy link
Contributor

@kwypchlo kwypchlo left a comment

Choose a reason for hiding this comment

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

all tests that have it.each should have %s in their name that interpolates the input string so the tests have unique names, otherwise we end up with 300 tests with the same name on each of such case

it.each(invalidCases)("should return null on invalid case %s", (fullSkylink) => {

src/download.test.ts Outdated Show resolved Hide resolved
src/download.ts Outdated Show resolved Hide resolved
@mrcnski mrcnski requested a review from kwypchlo November 18, 2020 16:44
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants