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.

Sign up for GitHub

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

Jump to bottom

Add partialUpdateFileContents method #362

Merged
merged 3 commits into from
Mar 18, 2024
Merged

Add partialUpdateFileContents method #362

merged 3 commits into from
Mar 18, 2024

Conversation

ArcticLampyrid
Copy link
Contributor

This pull request adds a new method called partialUpdateFileContents to the WebDAVClient class. This method allows for updating a remote file with a partial update, which is useful for making changes to a file without having to download and re-upload the entire file.

Moreover, it can be used to implement chunked file uploading.

This will close #303.

n-peugnet
n-peugnet reviewed Jan 11, 2024
View reviewed changes
README.md Show resolved Hide resolved
n-peugnet added a commit to club-1/webdav-server that referenced this pull request Jan 11, 2024
@n-peugnet
Prepare support for partial updates
807b000 
As webdav-client may support it one day. And thus WebDAV-Drive also,
when we manage to implement the upload progress over fetch.

See: perry-mitchell/webdav-client#362, club-1/webdav-drive#65
perry-mitchell
perry-mitchell requested changes Feb 25, 2024
View reviewed changes
Copy link
Owner

@perry-mitchell perry-mitchell left a comment

Choose a reason for hiding this comment

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

Some minor items..

source/operations/getDavCompliance.ts Outdated Show resolved Hide resolved
source/operations/getDavCompliance.ts Outdated Show resolved Hide resolved
source/factory.ts Outdated Show resolved Hide resolved
source/operations/partialUpdateFileContents.ts Outdated Show resolved Hide resolved
source/operations/getDavCompliance.ts Outdated Show resolved Hide resolved
source/operations/partialUpdateFileContents.ts Outdated Show resolved Hide resolved
source/types.ts Outdated Show resolved Hide resolved
@perry-mitchell
Copy link
Owner

Awesome PR here! Thanks @ArcticLampyrid. The DAV compliance fetcher is very nice and I imagine will come in super useful in the future.

Some minor items in the PR, like naming etc.

@perry-mitchell
Copy link
Owner

@ArcticLampyrid Tests failing due to drop in coverage. Would you mind seeing if there are some lines added in this PR that are not covered by tests? And if so, adding coverage for them?

@ArcticLampyrid
Copy link
Contributor Author

ArcticLampyrid commented Feb 25, 2024 •
edited
Loading

The decrease in test coverage is expected. This PR adds partialUpdateFileContents which relies on a non-standard extension, and the webdav-server used during testing does not yet support this extension. In my opinion, there is no simple way to add complete tests. The current unit tests only cover the "not support" scenario.

@perry-mitchell
Copy link
Owner

Ok. Would you mind then dropping the coverage amounts to match the new values? I'll merge after that and once the tests pass.

@ArcticLampyrid
Copy link
Contributor Author

Would you mind then dropping the coverage amounts to match the new values?

Of course, I'd be happy to adjust the coverage amounts to align with the new values.

@perry-mitchell
Copy link
Owner

Sorry this took so long to move. Merging.

@perry-mitchell perry-mitchell merged commit b06f2de into perry-mitchell:master Mar 18, 2024
6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@n-peugnet n-peugnet n-peugnet left review comments

@perry-mitchell perry-mitchell perry-mitchell approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Is it possible to append a new line to file on webdav server?
3 participants
@ArcticLampyrid @perry-mitchell @n-peugnet