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

Add files affected by a commit to gitea API -- similar to github #12413

Closed
wants to merge 10 commits into from

Conversation

jasugun
Copy link
Contributor

@jasugun jasugun commented Aug 3, 2020

Hi,

The idea is to have ou cis tool track files affected from polled changes at compile time for our gitea projects - as currently possible with github projects.
The current API returns a whole commit tree instead of a simple list of affected files. We'd like to implement this list, just like in github.

Thanks,
Laurent Cahour

@lafriks
Copy link
Member

lafriks commented Aug 3, 2020

Please make fmt to fix lint problems

@GiteaBot GiteaBot added the lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. label Aug 3, 2020
@lafriks lafriks added modifies/api This PR adds API routes or modifies them type/enhancement An improvement of existing functionality labels Aug 3, 2020
@lafriks lafriks added this to the 1.13.0 milestone Aug 3, 2020
@lafriks
Copy link
Member

lafriks commented Aug 3, 2020

Also you need to update swagger definition

@techknowlogick
Copy link
Member

For any reviewer looking for GH docs, they can be found here: https://docs.github.com/en/rest/reference/repos#get-a-commit

@lafriks
Copy link
Member

lafriks commented Aug 3, 2020

But if I understand correctly github returns list of objects not strings

modules/structs/repo_commit.go Outdated Show resolved Hide resolved
@lunny lunny modified the milestones: 1.13.0, 1.14.0 Sep 10, 2020
@jasugun
Copy link
Contributor Author

jasugun commented Sep 17, 2020

Hi there, I've psuhed a new iteration:

  • Fixed missing swagger template (forgot to add in the first place)
  • Did go fmt the pushed files.
  • Gone through the github doc and now use a response the resembles github.

I just include the filenames for now. We can always add the remaining stuff later (status, etc...)
adds the following to my test response : "files":[{"filename":"README.md"}]

Thanks for your time.
Laurent

@jasugun
Copy link
Contributor Author

jasugun commented Sep 17, 2020

Still not good for the lint sorry. Let me fix that.

@codecov-commenter
Copy link

codecov-commenter commented Sep 17, 2020

Codecov Report

Merging #12413 into master will increase coverage by 0.00%.
The diff coverage is 80.00%.

Impacted file tree graph

@@           Coverage Diff           @@
##           master   #12413   +/-   ##
=======================================
  Coverage   43.09%   43.09%           
=======================================
  Files         658      658           
  Lines       72454    72464   +10     
=======================================
+ Hits        31221    31229    +8     
  Misses      36178    36178           
- Partials     5055     5057    +2     
Impacted Files Coverage Δ
modules/convert/git_commit.go 87.60% <80.00%> (-0.69%) ⬇️
modules/process/manager.go 72.50% <0.00%> (-2.50%) ⬇️
modules/log/event.go 57.54% <0.00%> (-1.89%) ⬇️
modules/queue/workerpool.go 60.00% <0.00%> (+1.22%) ⬆️
modules/util/timer.go 85.71% <0.00%> (+42.85%) ⬆️

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 63e8bda...3cbcd39. Read the comment docs.

@jasugun
Copy link
Contributor Author

jasugun commented Sep 23, 2020

Hi there,

Can your guys please take a look and tell me if this is acceptable or not?

Thanks a lot for your time,
Laurent

@6543
Copy link
Member

6543 commented Sep 23, 2020

now it will not colide and can go in as is :)

only one thing: we need tests to make sure it wont break in future :)

thanks for the work 👍

@jasugun
Copy link
Contributor Author

jasugun commented Sep 23, 2020

Thanks a lot for the quick response!

@6543
Copy link
Member

6543 commented Nov 19, 2020

@jasugun do you need help writting tests?

@stale
Copy link

stale bot commented Jan 19, 2021

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.

@stale stale bot added the issue/stale label Jan 19, 2021
@6543 6543 added the issue/confirmed Issue has been reviewed and confirmed to be present or accepted to be implemented label Jan 19, 2021
@stale stale bot removed the issue/stale label Jan 19, 2021
@6543
Copy link
Member

6543 commented Jan 19, 2021

@jasugun can you resolve the conflict?

@zeripath
Copy link
Contributor

... or allow maintainers to update the PR to do it for you

@6543
Copy link
Member

6543 commented Feb 2, 2021

... or allow maintainers to update the PR to do it for you

not possible due to forked to an org

@6543
Copy link
Member

6543 commented Feb 5, 2021

followup: #14579

@6543 6543 closed this Feb 5, 2021
@go-gitea go-gitea locked and limited conversation to collaborators Mar 11, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
issue/confirmed Issue has been reviewed and confirmed to be present or accepted to be implemented lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. modifies/api This PR adds API routes or modifies them type/enhancement An improvement of existing functionality
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants