Adds Parent property to the repo API #1687

guyzmo · 2017-05-05T19:07:32Z

hop! 👍

lafriks · 2017-05-05T19:22:17Z

@guyzmo you need to create PR to https://github.com/go-gitea/go-sdk to add required field

guyzmo · 2017-05-05T20:07:17Z

done

guyzmo · 2017-05-05T21:27:34Z

I just changed the content, to expose Repository* BaseRepo as 'parent' property, instead of an id. because that's more restful.

guyzmo · 2017-05-05T22:03:34Z

cf github API, the parent is a full object. The only difference is that the parent does not contain its own parent in the project's tree (to avoid blowing up the connection). Also there's a source attribute, that contain the upmost parent project of the tree.

I guess that we can start iteratively, and give the parent property as it is currently implemented, at the risk of blowing the connection in case the project tree's branch is huge. Then in a second stage, hack the parent property so that it does not contain itself another parent property, or at least is set to null (and thus fork=true + parent=null means that there's more).

lafriks · 2017-05-05T23:12:24Z

This code will panic when BaseRepo is nil. Also I'm not quite sure if BaseRepo will be have value at all before GetBaseRepo is called

guyzmo · 2017-05-06T11:08:14Z

👌, so we need the code to keep calm when BaseRepo is nil. How?
And also we need to make sure that BaseRepo is valued when the API needs it. How?

I don't know the code base enough to tell how to hack that, so any pointers are welcomed ☺

lunny · 2017-05-06T11:33:06Z

@guyzmo first at all, you should use govendor update code.gitea.io/sdk to update the dependency.

lafriks · 2017-05-07T16:50:44Z

Now that dependency is set correctly you can see that unit tests is failing where BaseRepo is nil

guyzmo · 2017-05-07T20:42:09Z

So what can be done, how can we make sure it's 👌 when the pointer is nil and also that a tentative is made to resolve the parent when not?

lafriks · 2017-05-07T22:12:46Z

you need to move out assignment up something like:

var parent *api.Repository
if repo.BaseRepo != nil {
    parent = repo.BaseRepo.APIFormat(AccessModeNone)
}

guyzmo · 2017-05-07T22:35:48Z

and to make sure the repo.BaseRepo contains something?

lafriks · 2017-05-08T15:29:58Z

Then just assign Parent: parent,

lafriks · 2017-05-08T15:32:31Z

If you meant to actually load BaseRepo than it should be done outside APIFormat in function from where APIFormat is called

guyzmo · 2017-05-08T16:12:57Z

ok, I've tried something to force load the BaseRepo. The build is failing, but it doesn't look like related to that code I wrote, but I might be wrong.

lafriks · 2017-05-08T21:01:11Z

models/repo.go

+		log.Error(4, "GetRepositoryByID[%d]: %v", id, err)
+	}
+	if repo.BaseRepo != nil {
+		parent := repo.BaseRepo.APIFormat(AccessModeNone)


it must be parent = repo.BaseRepo...

and I guess parent := nil as well. My bad! 👍

lafriks · 2017-05-10T06:24:02Z

models/repo.go

 	cloneLink := repo.CloneLink()
 	permission := &api.Permission{
 		Admin: mode >= AccessModeAdmin,
 		Push:  mode >= AccessModeWrite,
 		Pull:  mode >= AccessModeRead,
 	}
+	err := repo.GetBaseRepo()


if isParent is true no need to try to call GetBaseRepo as it's result will not be used anyway

bkcsoft · 2017-05-10T07:43:24Z

models/repo.go

 	cloneLink := repo.CloneLink()
 	permission := &api.Permission{
 		Admin: mode >= AccessModeAdmin,
 		Push:  mode >= AccessModeWrite,
 		Pull:  mode >= AccessModeRead,
 	}
+	err := repo.GetBaseRepo()
+	if err != nil {
+		log.Error(4, "APIFormat issue: %v", err)


just "APIFormat: %v", err ;)

bkcsoft · 2017-05-10T12:16:25Z

now it's LGTM 🎉

lafriks · 2017-05-10T12:17:35Z

LGTM

lunny · 2017-05-11T07:36:17Z

models/repo.go

@@ -272,12 +272,26 @@ func (repo *Repository) APIURL() string {

 // APIFormat converts a Repository to api.Repository
 func (repo *Repository) APIFormat(mode AccessMode) *api.Repository {
+	return repo.innerAPIFormat(mode, false)
+}
+func (repo *Repository) innerAPIFormat(mode AccessMode, isParent bool) *api.Repository {


Signed-off-by: Guyzmo <[email protected]>

lunny · 2017-05-12T00:55:49Z

let L-G-T-M work

changes introduced to repo format by : go-gitea#1687 go-gitea/go-sdk#58 go-gitea#1668 go-gitea/go-sdk#56

changes introduced to repo format by : #1687 go-gitea/go-sdk#58 #1668 go-gitea/go-sdk#56

tboerger added the lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. label May 5, 2017

guyzmo mentioned this pull request May 5, 2017

Added ForkID property to the repo API go-gitea/go-sdk#58

Closed

This was referenced May 5, 2017

Added Parent property to the repo API go-gitea/go-sdk#59

Merged

Adds parent property of repo endpoint unfoldingWord-dev/python-gitea-client#12

Merged

guyzmo force-pushed the feature/api_exposes_forkid_property branch from 93beb0f to 9d5ce90 Compare May 5, 2017 21:19

guyzmo changed the title ~~Adds ForkID property to the repo API~~ Adds Parent property to the repo API May 5, 2017

guyzmo force-pushed the feature/api_exposes_forkid_property branch from 9d5ce90 to e879a31 Compare May 5, 2017 21:45

lunny added this to the 1.2.0 milestone May 6, 2017

lunny added modifies/api This PR adds API routes or modifies them type/enhancement An improvement of existing functionality labels May 6, 2017

guyzmo force-pushed the feature/api_exposes_forkid_property branch 3 times, most recently from 2541322 to 683316b Compare May 6, 2017 16:19

guyzmo force-pushed the feature/api_exposes_forkid_property branch from 683316b to 7565d77 Compare May 8, 2017 15:49

guyzmo force-pushed the feature/api_exposes_forkid_property branch from 7565d77 to fb60313 Compare May 8, 2017 20:15

lafriks requested changes May 8, 2017

View reviewed changes

tboerger added lgtm/need 1 This PR needs approval from one additional maintainer to be merged. and removed lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. labels May 9, 2017

guyzmo force-pushed the feature/api_exposes_forkid_property branch 7 times, most recently from 4cf7027 to 5567d85 Compare May 9, 2017 20:52

lafriks requested changes May 10, 2017

View reviewed changes

bkcsoft suggested changes May 10, 2017

View reviewed changes

guyzmo force-pushed the feature/api_exposes_forkid_property branch 2 times, most recently from 25875cf to 7f67717 Compare May 10, 2017 12:15

bkcsoft approved these changes May 10, 2017

View reviewed changes

tboerger added lgtm/done This PR has enough approvals to get merged. There are no important open reservations anymore. and removed lgtm/need 1 This PR needs approval from one additional maintainer to be merged. labels May 10, 2017

lafriks approved these changes May 10, 2017

View reviewed changes

lunny reviewed May 11, 2017

View reviewed changes

Added Parent property to the repo API

a541d73

Signed-off-by: Guyzmo <[email protected]>

guyzmo force-pushed the feature/api_exposes_forkid_property branch from 7f67717 to a541d73 Compare May 11, 2017 07:40

appleboy approved these changes May 12, 2017

View reviewed changes

appleboy merged commit 397474e into go-gitea:master May 12, 2017

sapk mentioned this pull request May 14, 2017

Add public/swagger.v1.json to .gitignore? #1719

Closed

7 tasks

sapk added a commit to sapk-fork/gitea that referenced this pull request May 14, 2017

re-Generate swagger.json

3127047

changes introduced to repo format by : go-gitea#1687 go-gitea/go-sdk#58 go-gitea#1668 go-gitea/go-sdk#56

sapk mentioned this pull request May 14, 2017

Fix #1719 #1722

Merged

appleboy pushed a commit that referenced this pull request May 14, 2017

re-Generate swagger.json (#1722)

91417e9

changes introduced to repo format by : #1687 go-gitea/go-sdk#58 #1668 go-gitea/go-sdk#56

go-gitea locked and limited conversation to collaborators Nov 23, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Adds Parent property to the repo API #1687

Adds Parent property to the repo API #1687

guyzmo commented May 5, 2017 •

edited

Loading

lafriks commented May 5, 2017 •

edited

Loading

guyzmo commented May 5, 2017

guyzmo commented May 5, 2017

guyzmo commented May 5, 2017

lafriks commented May 5, 2017

guyzmo commented May 6, 2017

lunny commented May 6, 2017

lafriks commented May 7, 2017

guyzmo commented May 7, 2017

lafriks commented May 7, 2017

guyzmo commented May 7, 2017

lafriks commented May 8, 2017

lafriks commented May 8, 2017

guyzmo commented May 8, 2017

lafriks May 8, 2017

guyzmo May 8, 2017

lafriks May 10, 2017 •

edited

Loading

bkcsoft May 10, 2017

guyzmo May 10, 2017

bkcsoft May 10, 2017

guyzmo May 10, 2017

bkcsoft commented May 10, 2017

lafriks commented May 10, 2017

lunny May 11, 2017

guyzmo May 11, 2017

lunny commented May 12, 2017

Adds Parent property to the repo API #1687

Adds Parent property to the repo API #1687

Conversation

guyzmo commented May 5, 2017 • edited Loading

lafriks commented May 5, 2017 • edited Loading

guyzmo commented May 5, 2017

guyzmo commented May 5, 2017

guyzmo commented May 5, 2017

lafriks commented May 5, 2017

guyzmo commented May 6, 2017

lunny commented May 6, 2017

lafriks commented May 7, 2017

guyzmo commented May 7, 2017

lafriks commented May 7, 2017

guyzmo commented May 7, 2017

lafriks commented May 8, 2017

lafriks commented May 8, 2017

guyzmo commented May 8, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

lafriks May 10, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bkcsoft commented May 10, 2017

lafriks commented May 10, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

lunny commented May 12, 2017

guyzmo commented May 5, 2017 •

edited

Loading

lafriks commented May 5, 2017 •

edited

Loading

lafriks May 10, 2017 •

edited

Loading