Skip to content

Conversation

@lucasqueiroz
Copy link
Contributor

Description

This Pull Request aims to solve the problem described on #13004, adding a timestamp to the tag list API.
As this is my first time writing Go code, please let me know if there's anything that could be improved.
I did not find any unit tests for the changed object.

Results

When running the API locally, the swagger docs result in this:

Screen Shot 2020-10-03 at 14 34 05

And the API returns the following information:

Screen Shot 2020-10-03 at 14 34 21

@codecov-commenter
Copy link

codecov-commenter commented Oct 3, 2020

Codecov Report

Merging #13026 into master will decrease coverage by 0.00%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master   #13026      +/-   ##
==========================================
- Coverage   42.55%   42.54%   -0.01%     
==========================================
  Files         672      672              
  Lines       73715    73716       +1     
==========================================
- Hits        31367    31361       -6     
- Misses      37264    37272       +8     
+ Partials     5084     5083       -1     
Impacted Files Coverage Δ
modules/convert/git_commit.go 88.39% <100.00%> (+0.10%) ⬆️
modules/indexer/stats/db.go 52.17% <0.00%> (-8.70%) ⬇️
services/pull/check.go 44.61% <0.00%> (-3.85%) ⬇️
models/unit.go 46.57% <0.00%> (-2.74%) ⬇️
routers/repo/view.go 37.47% <0.00%> (-0.65%) ⬇️
models/error.go 34.34% <0.00%> (-0.51%) ⬇️
services/pull/pull.go 41.27% <0.00%> (+0.49%) ⬆️
modules/queue/unique_queue_disk_channel.go 55.38% <0.00%> (+1.53%) ⬆️
modules/log/event.go 59.43% <0.00%> (+1.88%) ⬆️

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 48703c3...45be31e. Read the comment docs.

@GiteaBot GiteaBot added the lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. label Oct 3, 2020
@6543
Copy link
Member

6543 commented Oct 4, 2020

Pleace add a test :)

@lucasqueiroz
Copy link
Contributor Author

@6543 I've added unit tests for the ToCommitMeta method, not sure if the approach I took is the best, but I couldn't find any way to load Tags from a loaded bean.
Please let me know if anything can be improved

@GiteaBot GiteaBot 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 Oct 4, 2020
@lafriks lafriks added the modifies/api This PR adds API routes or modifies them label Oct 4, 2020
@lafriks lafriks added this to the 1.13.0 milestone Oct 4, 2020
@6543
Copy link
Member

6543 commented Oct 4, 2020

@lucasqueiroz just make fmt :)

@lucasqueiroz
Copy link
Contributor Author

@lucasqueiroz just make fmt :)

Whoops my bad!

@GiteaBot GiteaBot 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 Oct 5, 2020
@lunny lunny merged commit 67a5573 into go-gitea:master Oct 5, 2020
@lucasqueiroz lucasqueiroz deleted the 13004/add-timestamp-to-tag-commit-information branch October 5, 2020 07:40
@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/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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants