Use /orgs instead of /org to match GitHub v3 API #2639

raphink · 2017-10-02T13:14:06Z

As per https://developer.github.com/v3/repos/#create

codecov-io · 2017-10-02T13:28:09Z

Codecov Report

Merging #2639 into master will increase coverage by 0.01%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##           master    #2639      +/-   ##
==========================================
+ Coverage   27.11%   27.12%   +0.01%     
==========================================
  Files          86       86              
  Lines       17064    17064              
==========================================
+ Hits         4627     4629       +2     
+ Misses      11759    11757       -2     
  Partials      678      678

Impacted Files	Coverage Δ
models/repo_unit.go	`8% <0%> (ø)`	⬆️
models/repo.go	`13.14% <0%> (+0.01%)`	⬆️
models/user.go	`18.24% <0%> (+0.17%)`	⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update a8717e5...e7bb20b. Read the comment docs.

sapk · 2017-10-02T14:28:41Z

routers/api/v1/repo/repo.go

@@ -173,7 +173,7 @@ func CreateOrgRepo(ctx *context.APIContext, opt api.CreateRepoOption) {
 	//       403: forbidden
 	//       500: error


You need to update the swagger:route (the url part) also. You can even edit {org} since in url params in swagger are not yet configured.

sapk · 2017-10-02T14:33:25Z

The cherry on the cake would be to keep the old url available and mark it as deprecated in swagger.

Keeping the old m.Post("/org/:orgname/repos", reqToken(), bind(api.CreateRepoOption{}), repo.CreateOrgRepo)
Add an other (duplicate) swagger comment with deprected tag inside CreateOrgRepo.

strk · 2017-10-02T15:25:29Z

But you're doing this under the /v1/ group, shouldn't it go to /v3/ group instead ?
Those versions are there for a reason...

lafriks · 2017-10-02T18:08:52Z

I agree with @strk that v1 route should be left as it is and note added in swagger docs that it is deprecated and new one should be created under v3 that would be with GitHub compatible route

raphink · 2017-10-03T07:45:12Z

@lafriks @strk Agreed, but I don't see a v3 in routers/api. Am I missing something?

lafriks · 2017-10-03T16:36:43Z

@raphink no currently there is no such route. But you can add it ;)

sapk · 2017-10-04T08:31:27Z

routers/api/v1/repo/repo.go

@@ -159,7 +159,7 @@ func Create(ctx *context.APIContext, opt api.CreateRepoOption) {

 // CreateOrgRepo create one repository of the organization
 func CreateOrgRepo(ctx *context.APIContext, opt api.CreateRepoOption) {
-	// swagger:route POST /org/{org}/repos organization createOrgRepo
+	// swagger:route POST /org/{orgname}/repos organization createOrgRepo


/orgs/{orgname}/repos

Ah, thanks!

jonasfranz

I think this PR needs the BREAKING label since it changes the API urls.

lafriks · 2017-10-05T10:50:03Z

@JonasFranzDEV that's why I think this should be done as new /v3 route and current marked as deprecated not to break existing API

strk · 2017-10-05T11:00:33Z

Didn't we say that a new endpoint should have been added (/v3/) instead of changing any of the existing ones ?

bkcsoft · 2017-10-10T20:39:54Z

Since we're not compatible with GitHub v3 we shouldn't put things under /v3/ until we are.

raphink · 2017-10-11T15:59:37Z

Currently, my issue is to be able to use the Terraform github provider with gitea, which is currently not possible when creating repositories (due to that difference in paths).

strk · 2017-10-11T16:32:11Z

@bkc why coulnd't we just put partial support /v3/ while it's being implemented ? As long as the /v2 still works ?

lunny · 2017-10-12T06:27:15Z

Put the routers to /api/v3 and keep old router?

daviian · 2017-10-12T06:33:45Z

I guess implementing v3 isn't done in a single PR. So if nobody starts with it, we will never implemented it.

lunny · 2017-10-12T06:37:26Z

Or keep both routers for compatibility since there are no conflicts between them.

daviian · 2017-10-12T06:40:51Z

Wasn't against that suggestion. We should keep both and after v3 is finished we should fix an end date of v1 support

bkcsoft · 2017-10-15T06:44:01Z

The current API uses v1, adding a v3-router is out or scope for this PR. (But please feel free to create a new one)

stale · 2019-01-05T13:52:57Z

This pull request has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs during the next 2 months. Thank you for your contributions.

techknowlogick · 2019-01-23T18:13:25Z

Closing per above discussion. Once we get a /v3/ route for API that is compatible with GitHub, then we can re-open this PR.

Thanks for contribution.

Use /orgs instead of /org to match GitHub v3 API

45f9c23

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

Use :orgname instead of :org for doc consistency

0a51e10

sapk suggested changes Oct 2, 2017

View reviewed changes

Fix swagger:route

8595318

sapk suggested changes Oct 4, 2017

View reviewed changes

Missing s in swagger:route

e7bb20b

jonasfranz approved these changes Oct 5, 2017

View reviewed changes

lunny added the pr/breaking Merging this PR means builds will break. Needs a description what exactly breaks, and how to fix it! label Oct 7, 2017

lafriks mentioned this pull request Dec 3, 2017

Add API v3 routes for compitable with github #3076

Closed

stale bot added the issue/stale label Jan 5, 2019

bkcsoft 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 Jan 5, 2019

stale bot removed issue/stale labels Jan 5, 2019

techknowlogick closed this Jan 23, 2019

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

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Use /orgs instead of /org to match GitHub v3 API #2639

Use /orgs instead of /org to match GitHub v3 API #2639

raphink commented Oct 2, 2017

codecov-io commented Oct 2, 2017 •

edited

Loading

sapk Oct 2, 2017 •

edited

Loading

sapk commented Oct 2, 2017 •

edited

Loading

strk commented Oct 2, 2017

lafriks commented Oct 2, 2017

raphink commented Oct 3, 2017

lafriks commented Oct 3, 2017

sapk Oct 4, 2017

raphink Oct 4, 2017

jonasfranz left a comment

lafriks commented Oct 5, 2017

strk commented Oct 5, 2017

bkcsoft commented Oct 10, 2017

raphink commented Oct 11, 2017

strk commented Oct 11, 2017 via email

lunny commented Oct 12, 2017

daviian commented Oct 12, 2017

lunny commented Oct 12, 2017

daviian commented Oct 12, 2017

bkcsoft commented Oct 15, 2017

stale bot commented Jan 5, 2019

techknowlogick commented Jan 23, 2019

		@@ -173,7 +173,7 @@ func CreateOrgRepo(ctx *context.APIContext, opt api.CreateRepoOption) {
		// 403: forbidden
		// 500: error

Use /orgs instead of /org to match GitHub v3 API #2639

Use /orgs instead of /org to match GitHub v3 API #2639

Conversation

raphink commented Oct 2, 2017

codecov-io commented Oct 2, 2017 • edited Loading

Codecov Report

sapk Oct 2, 2017 • edited Loading

Choose a reason for hiding this comment

sapk commented Oct 2, 2017 • edited Loading

strk commented Oct 2, 2017

lafriks commented Oct 2, 2017

raphink commented Oct 3, 2017

lafriks commented Oct 3, 2017

sapk Oct 4, 2017

Choose a reason for hiding this comment

raphink Oct 4, 2017

Choose a reason for hiding this comment

jonasfranz left a comment

Choose a reason for hiding this comment

lafriks commented Oct 5, 2017

strk commented Oct 5, 2017

bkcsoft commented Oct 10, 2017

raphink commented Oct 11, 2017

strk commented Oct 11, 2017 via email

lunny commented Oct 12, 2017

daviian commented Oct 12, 2017

lunny commented Oct 12, 2017

daviian commented Oct 12, 2017

bkcsoft commented Oct 15, 2017

stale bot commented Jan 5, 2019

techknowlogick commented Jan 23, 2019

codecov-io commented Oct 2, 2017 •

edited

Loading

sapk Oct 2, 2017 •

edited

Loading

sapk commented Oct 2, 2017 •

edited

Loading