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

GitHub API Compliance Fixes #227

Merged
merged 3 commits into from
Dec 2, 2016
Merged

GitHub API Compliance Fixes #227

merged 3 commits into from
Dec 2, 2016

Conversation

bkcsoft
Copy link
Member

@bkcsoft bkcsoft commented Nov 23, 2016

This fixes a few API-bugs

Fixes!

  • Label-data are now compliant
  • Labels can be fetched by Name (as well as ID)
  • API-Token can be sent via ?access_token=[...]
  • Issue-Listing can be queried for ?state=all to fetch all issues (for said repo)
  • Repos can be fetched by ID with /repositories/:id (undocumented feature, but it's used by OctoKit 😒 )

Backwards Compatability Breakage!

  • label.Color is no longer prefixed with # (since GH doesn't do that)

Related Issue: #64
I'm sure there are more Gogs-issues that I'm closing with this PR, but I'm to lazy ATM to look them up 😆

@bkcsoft bkcsoft added this to the 1.1.0 milestone Nov 23, 2016
@thibaultmeyer
Copy link
Contributor

LGTM

@tboerger tboerger added the lgtm/need 1 This PR needs approval from one additional maintainer to be merged. label Nov 23, 2016
@ghost
Copy link

ghost commented Nov 23, 2016

LGTM.

@tboerger tboerger added the lgtm/done This PR has enough approvals to get merged. There are no important open reservations anymore. label Nov 23, 2016
@bkcsoft bkcsoft removed the lgtm/need 1 This PR needs approval from one additional maintainer to be merged. label Nov 23, 2016
@bkcsoft
Copy link
Member Author

bkcsoft commented Nov 23, 2016

@lunny @tboerger I want a 3rd opinion since I'm actually breaking stuff here as well :trollface:

@tboerger tboerger added the lgtm/done This PR has enough approvals to get merged. There are no important open reservations anymore. label Nov 23, 2016
@ghost
Copy link

ghost commented Nov 23, 2016

@bkcsoft would it not be technically a 4th and 5th opinion? 1st being yours, 2nd being @0xBAADF00D and 3rd being me... unless I'm not an opinion ._.

@tboerger tboerger added the lgtm/done This PR has enough approvals to get merged. There are no important open reservations anymore. label Nov 23, 2016
@bkcsoft
Copy link
Member Author

bkcsoft commented Nov 23, 2016

@LefsFlarey I didn't count myself 😆

@tboerger tboerger added the lgtm/done This PR has enough approvals to get merged. There are no important open reservations anymore. label Nov 23, 2016
@ghost
Copy link

ghost commented Nov 23, 2016

@bkcsoft oh ok. .-.

@tboerger tboerger added the lgtm/done This PR has enough approvals to get merged. There are no important open reservations anymore. label Nov 23, 2016
@@ -244,6 +244,20 @@ func Get(ctx *context.APIContext) {
ctx.JSON(200, repo.APIFormat(&api.Permission{true, true, true}))
}

func GetByID(ctx *context.APIContext) {
repo, err := models.GetRepositoryByID(ctx.ParamsInt64(":id"))
Copy link
Member

Choose a reason for hiding this comment

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

golint

@codecov-io
Copy link

codecov-io commented Nov 24, 2016

Current coverage is 3.02% (diff: 0.00%)

Merging #227 into master will decrease coverage by <.01%

@@            master      #227   diff @@
========================================
  Files           33        33          
  Lines         8106      8120    +14   
  Methods          0         0          
  Messages         0         0          
  Branches         0         0          
========================================
  Hits           246       246          
- Misses        7840      7854    +14   
  Partials        20        20          

Powered by Codecov. Last update cb16028...b6ff427

@bkcsoft
Copy link
Member Author

bkcsoft commented Nov 24, 2016

@lunny done :)

@bkcsoft bkcsoft force-pushed the api/github-compliance branch from b6ff427 to b17506c Compare November 24, 2016 20:44
@bkcsoft
Copy link
Member Author

bkcsoft commented Nov 24, 2016

rebased and fixed conflicts :)

@bkcsoft bkcsoft modified the milestones: 1.0.0, 1.1.0 Nov 29, 2016
@bkcsoft
Copy link
Member Author

bkcsoft commented Nov 29, 2016

@lunny could you review again? 🙂

@tboerger
Copy link
Member

LGTM

@tboerger
Copy link
Member

But maybe you need to rebase as the drone push failed with matrix builds.

@bkcsoft
Copy link
Member Author

bkcsoft commented Nov 29, 2016

@tboerger noted, I also need to rebase now that a few fixes in this one got fixed in #300 (related to go-sdk vendor update)...

@lunny
Copy link
Member

lunny commented Nov 29, 2016

push failed

@bkcsoft bkcsoft force-pushed the api/github-compliance branch from b17506c to e6cfccd Compare December 2, 2016 08:18
@bkcsoft bkcsoft force-pushed the api/github-compliance branch from 682cd9a to e8e0539 Compare December 2, 2016 08:33
@bkcsoft
Copy link
Member Author

bkcsoft commented Dec 2, 2016

Now it should be fine :D

@bkcsoft bkcsoft merged commit d7ed78a into master Dec 2, 2016
@bkcsoft bkcsoft deleted the api/github-compliance branch December 2, 2016 09:04
@go-gitea go-gitea locked and limited conversation to collaborators Nov 23, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
lgtm/done This PR has enough approvals to get merged. There are no important open reservations anymore. type/bug
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants