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.

Sign up for GitHub

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

Jump to bottom

Implement git refs API for listing references (branches, tags and other) #5354

Merged
merged 9 commits into from
Nov 27, 2018
Merged

Implement git refs API for listing references (branches, tags and other) #5354

merged 9 commits into from
Nov 27, 2018

Conversation

lafriks
Copy link
Member

@lafriks lafriks commented Nov 18, 2018 •
edited
Loading

Implements git refs API /repos/{owner}/{repo}/git/refs/{ref}. Implements only GET methods fully compatible with GitHub API

Partly fixes #2738

Depends on go-gitea/go-sdk#129

TODO:

  • SDK pull request
  • Tests

@lafriks lafriks added type/feature Completely new functionality. Can only be merged if feature freeze is not active. modifies/api This PR adds API routes or modifies them labels Nov 18, 2018
@lafriks lafriks added this to the 1.7.0 milestone Nov 18, 2018
@lafriks lafriks mentioned this pull request Nov 18, 2018
Merged
@bkcsoft bkcsoft added the lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. label Nov 19, 2018
@techknowlogick
Copy link
Member

@lafriks please update vendored files, and resolve conflict.

@codecov-io
Copy link

codecov-io commented Nov 24, 2018 •
edited
Loading

Codecov Report

Merging #5354 into master will increase coverage by 0.11%.
The diff coverage is 91.57%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #5354      +/-   ##
==========================================
+ Coverage   37.46%   37.57%   +0.11%     
==========================================
  Files         312      313       +1     
  Lines       46522    46617      +95     
==========================================
+ Hits        17428    17515      +87     
- Misses      26601    26607       +6     
- Partials     2493     2495       +2
Impacted Files Coverage Δ
routers/api/v1/api.go 74.47% <100%> (+0.21%) ⬆️
routers/api/v1/repo/git_ref.go 91.2% <91.2%> (ø)

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 2949043...7e4defb. Read the comment docs.

@lafriks
Copy link
Member Author

lafriks commented Nov 24, 2018

Ready for review

lunny
lunny requested changes Nov 24, 2018
View reviewed changes
routers/api/v1/repo/git_ref.go
SHA: refs[i].Object.String(),
Type: refs[i].Type,
// TODO: Add commit/tag info URL
//URL: ctx.Repo.Repository.APIURL() + "/git/" + refs[i].Type + "s/" + refs[i].Object.String(),
Copy link
Member

Choose a reason for hiding this comment

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

Not finished?

Copy link
Member Author

Choose a reason for hiding this comment

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

That is for next pull request where I will implement getting tags details in route /git/tags

routers/api/v1/api.go
@@ -573,6 +573,10 @@ func RegisterRoutes(m *macaron.Macaron) {
m.Get("/status", repo.GetCombinedCommitStatusByRef)
m.Get("/statuses", repo.GetCommitStatusesByRef)
})
m.Group("/git", func() {
Copy link
Member

Choose a reason for hiding this comment

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

permission check?

Copy link
Member Author

Choose a reason for hiding this comment

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

what exact permission check are you talking about. (It is checked for access read only rights for repo). Do you mean unit type check?

Copy link
Member

Choose a reason for hiding this comment

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

Didn't find the access read check. Unit type check should be fixed by my PR #5314 .

Copy link
Member Author

Choose a reason for hiding this comment

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

It is already checked in upper group in function repoAssignment

Copy link
Member

Choose a reason for hiding this comment

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

Right. It's different from the middleware which has the same name and used by ui routers.

@lafriks
Copy link
Member Author

lafriks commented Nov 26, 2018

@lunny ? :)

@lafriks
Copy link
Member Author

lafriks commented Nov 26, 2018

@lunny so is there anything I need to fix this PR?

@bkcsoft 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 Nov 26, 2018
@bkcsoft bkcsoft 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 Nov 27, 2018
@techknowlogick techknowlogick merged commit 08bf443 into go-gitea:master Nov 27, 2018
@techknowlogick techknowlogick added the type/changelog Adds the changelog for a new Gitea version label Nov 27, 2018
@lafriks lafriks deleted the feat/tags_api branch November 27, 2018 22:37
@go-gitea go-gitea locked and limited conversation to collaborators Nov 24, 2020
@delvh delvh removed the type/changelog Adds the changelog for a new Gitea version label Oct 7, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@adelowo adelowo adelowo left review comments

@techknowlogick techknowlogick techknowlogick approved these changes

@lunny lunny lunny approved these changes

Assignees
No one assigned
Labels
lgtm/done This PR has enough approvals to get merged. There are no important open reservations anymore. 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
Milestone
1.7.0
Development

Successfully merging this pull request may close these issues.

API support for listing tags and fetching tag details
7 participants
@lafriks @techknowlogick @codecov-io @lunny @adelowo @bkcsoft @delvh