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

[WIP] feature: list files in tree #2749

Closed
wants to merge 10 commits into from
Closed

[WIP] feature: list files in tree #2749

wants to merge 10 commits into from

Conversation

necaris
Copy link

@necaris necaris commented Oct 20, 2017

Includes code stolen from @dsludwig (see dsludwig@68edeb4)

Closes #1978

Note this is still very incomplete and needs design discussion, which (as I understand it) should be happening on the issue?

@lunny lunny added modifies/api This PR adds API routes or modifies them type/feature Completely new functionality. Can only be merged if feature freeze is not active. labels Oct 21, 2017
@lunny lunny added this to the 1.x.x milestone Oct 21, 2017
@@ -1,8 +1,79 @@
// TODO: Create TreeListing and RepoFile and other model objects here, and then
Copy link
Member

Choose a reason for hiding this comment

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

This file should be put in github.com/go-gitea/git not here.

Copy link
Author

Choose a reason for hiding this comment

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

@lunny these types are for the API representation of the tree listing, matching the GitHub API, and to make the tree listing API easier to test -- otherwise they add little to the git.Tree and git.TreeEntry types that already exist. Would it be better to avoid defining public types, and instead add JSON annotations to those types in go-gitea/git ?

Copy link
Member

Choose a reason for hiding this comment

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

If we're sending them over to the Client they should be defined in go-gitea/go-sdk 🙂

// RepoFile represents a file blob contained in the repository
type RepoFile struct {
Path string `json:"path"`
// Mode git.EntryMode `json:"mode"` // TODO: Do we include this? It'll require exporting the mode as public in the `git` module...
Copy link
Member

Choose a reason for hiding this comment

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

How does GitHub API represent them? usually we create a APIFormat function for things like this

Path string `json:"path"`
// Mode git.EntryMode `json:"mode"` // TODO: Do we include this? It'll require exporting the mode as public in the `git` module...
Type git.ObjectType `json:"type"`
// Size int64 `json:"size"` // TODO: Do we include this? It's expensive...
Copy link
Member

Choose a reason for hiding this comment

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

It's not though, since git cat-file prints the size along with the ref and filename :/

@HoffmannP
Copy link
Contributor

Is anyone currently working on this?

@bkcsoft bkcsoft added the lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. label Nov 18, 2018
@lunny
Copy link
Member

lunny commented Nov 19, 2018

@HoffmannP I think no since this PR doesn't update for 1 month. @necaris are you still working on this?

@necaris
Copy link
Author

necaris commented Nov 19, 2018 via email

@lunny
Copy link
Member

lunny commented Nov 25, 2018

@necaris anothr PR #4185 also try to fix the issue. If you have too much time, maybe you could give some review on that and close this one?

@necaris
Copy link
Author

necaris commented Nov 25, 2018

@HoffmannP FYI, as @lunny suggested I'm going to close this in favor of #4185 which is already more complete -- looking at that code it looks like it's doing pretty much the same, but reinventing fewer data structures that have been added in the meantime. In case anyone finds this code useful I have rebased it to be up to date, though.

@necaris necaris closed this Nov 25, 2018
@lunny lunny removed this from the 1.x.x milestone Nov 25, 2018
@go-gitea go-gitea locked and limited conversation to collaborators Nov 24, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. modifies/api This PR adds API routes or modifies them type/feature Completely new functionality. Can only be merged if feature freeze is not active.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

API: file listing / tree access
4 participants