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

[VFS] Patch file meta #51

Merged
merged 5 commits into from
Oct 18, 2016
Merged

[VFS] Patch file meta #51

merged 5 commits into from
Oct 18, 2016

Conversation

jinroh
Copy link
Contributor

@jinroh jinroh commented Oct 17, 2016

Adds the PATCH /files/:file-id route to modify the metadata of a file, move it and rename it.

I've also added a vfs.Context struct to stop passing around fs / dbprefix values...

@jinroh jinroh changed the base branch from vfs-concurrent to master October 17, 2016 16:27
Copy link
Contributor

@aenario aenario left a comment

Choose a reason for hiding this comment

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

LGTM : one question is more curiosity and the other is for the future.

newtags = data.Tags
}

newdoc, err = NewFileDoc(
Copy link
Contributor

Choose a reason for hiding this comment

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

Why not keeping oldpath and modifying the old doc in place ?


// Context is used to convey the afero.Fs object along with the
// CouchDb database prefix.
type Context struct {
Copy link
Contributor

Choose a reason for hiding this comment

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

[Future] This could get complicated fast between gin.Context, couchdb.Prefix (+ some auth later), vfs.Context, middlewares.Instance. We should at least have some out-of-code doc on it, but better a proper discussion and normalisation.

@nono nono merged commit 162c4bb into master Oct 18, 2016
@nono nono deleted the vfs-patch-file-meta branch October 18, 2016 13:05
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