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

feat: directory uploading #208

Merged
merged 4 commits into from
Jan 19, 2023
Merged

Conversation

IgorShadurin
Copy link
Collaborator

@IgorShadurin IgorShadurin commented Jan 11, 2023

Close #205

Version for Node.js and browser. I failed to cover directory selection with real test. Perhaps it will be possible to cover with mocks.
Therefore, a page was created to manually test this functionality test/manual/index.html.
But the method will only work in the browser with fixes from this branch (duplex: half fix): #151.

@IgorShadurin IgorShadurin marked this pull request as ready for review January 16, 2023 13:25
@IgorShadurin IgorShadurin requested a review from nugaon as a code owner January 16, 2023 13:25
Copy link
Collaborator

@nugaon nugaon left a comment

Choose a reason for hiding this comment

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

Nice work, +1 for the unit tests! : )
Please check my comments but overall it seems good.

README.md Outdated Show resolved Hide resolved
src/directory/handler.ts Outdated Show resolved Hide resolved
src/directory/handler.ts Outdated Show resolved Hide resolved
src/utils/type.ts Outdated Show resolved Hide resolved
test/unit/directory/utils.spec.ts Outdated Show resolved Hide resolved
test/manual/js/script.js Show resolved Hide resolved
test/manual/js/script.js Outdated Show resolved Hide resolved
test/manual/index.html Show resolved Hide resolved
* @param accountData account data
* @param options upload options
*/
export async function uploadData(
Copy link
Collaborator

Choose a reason for hiding this comment

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

I see you moved this function from file.ts to here, but can you elaborate for me please what is the role of the handler.ts in the service folders and according to what you distingish to put functions to the other main file like here the file.ts?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

file/file.ts, directory/directory.ts could be renamed to file/index.ts, directory/index.ts.

So, this renamed index.ts is an aggregation of all functions into functions that are available to the user. They validate user input and pass it to the method from handler.ts. handler.ts aggregates a function from utils.ts and other files.

src/file/utils.ts Outdated Show resolved Hide resolved
@IgorShadurin IgorShadurin force-pushed the feat/205-directory-uploading branch from 9f726c3 to bedfb9b Compare January 19, 2023 16:04
@IgorShadurin IgorShadurin merged commit 4a3a9b9 into master Jan 19, 2023
@IgorShadurin IgorShadurin deleted the feat/205-directory-uploading branch January 19, 2023 17:13
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.

Directory uploading
2 participants