Add API endpoint to get changed files of a PR #18228

anbraten · 2022-01-10T16:21:58Z

closes #654

lunny · 2022-01-11T01:52:13Z

We need more fields here.

{
    "sha": "bbcd538c8e72b8c175046e27cc8f907076331401",
    "filename": "file1.txt",
    "status": "added",
    "additions": 103,
    "deletions": 21,
    "changes": 124,
    "blob_url": "https://github.com/octocat/Hello-World/blob/6dcb09b5b57875f334f61aebed695e2e4193db5e/file1.txt",
    "raw_url": "https://github.com/octocat/Hello-World/raw/6dcb09b5b57875f334f61aebed695e2e4193db5e/file1.txt",
    "contents_url": "https://api.github.com/repos/octocat/Hello-World/contents/file1.txt?ref=6dcb09b5b57875f334f61aebed695e2e4193db5e",
    "patch": "@@ -132,7 +132,7 @@ module Test @@ -1000,7 +1000,7 @@ module Test"
}

Gusted

Just a few touches on the code - also to addition on lunny, it should have more fields to become more useful. Also I'm not sure which applications might need this, so this could be really irrelevant. But it could be that applications might not need the patch field(which could become quite large on big PR's) so a option to not send that back would be nice in order to reduce network bandwidth for those who don't need it, but only need the other fields.

Gusted · 2022-01-11T04:40:59Z

routers/api/v1/repo/pull.go

+		return
+	}
+
+	var prInfo *git.CompareInfo


Maybe move this to Line 1321(after defer baseGitRepo.Close())

Gusted · 2022-01-11T04:42:38Z

routers/api/v1/repo/pull.go

+		&gitdiff.DiffOptions{
+			BeforeCommitID:     startCommitID,
+			AfterCommitID:      endCommitID,
+			SkipTo:             ctx.FormString("skip-to"),


skip-to isn't mentioned as query in the swagger description.

Gusted · 2022-01-11T04:47:41Z

routers/api/v1/repo/pull.go

+			MaxLines:           maxLines,
+			MaxLineCharacters:  setting.Git.MaxGitDiffLineCharacters,
+			MaxFiles:           maxFiles,
+			WhitespaceBehavior: gitdiff.GetWhitespaceFlag(ctx.Data["WhitespaceBehavior"].(string)),


This will always be empty as this ctx.Data is set by SetWhitespaceBehavior middleware.

Gusted · 2022-01-11T04:50:25Z

routers/api/v1/repo/pull.go

+			WhitespaceBehavior: gitdiff.GetWhitespaceFlag(ctx.Data["WhitespaceBehavior"].(string)),
+		}, ctx.FormStrings("files")...)
+	if err != nil {
+		ctx.ServerError("GetDiffRangeWithWhitespaceBehavior", err)


Suggested change

ctx.ServerError("GetDiffRangeWithWhitespaceBehavior", err)

ctx.ServerError("GetDiff", err)

Gusted · 2022-01-11T04:55:58Z

routers/api/v1/repo/pull.go

+	start, end := listOptions.GetStartEnd()
+
+	if end > totalNumberOfFiles {
+		end = totalNumberOfFiles


This could cause that it becomes. start > end In which a panic will be caused on line 1368 panic: runtime error: makeslice: cap out of range. Maybe add a check if start is still smaller than end?

Gusted · 2022-01-11T04:56:48Z

routers/api/v1/repo/pull.go

+		apiFile := convert.ToChangedFile(diff.Files[i])
+		apiFiles = append(apiFiles, apiFile)


Suggested change

apiFile := convert.ToChangedFile(diff.Files[i])

apiFiles = append(apiFiles, apiFile)

apiFiles = append(apiFiles, convert.ToChangedFile(diff.Files[i]))

6543 · 2022-01-14T04:00:58Z

routers/api/v1/api.go

@@ -930,6 +930,7 @@ func Routes(sessioner func(http.Handler) http.Handler) *web.Route {
 						m.Get(".{diffType:diff|patch}", repo.DownloadPullDiffOrPatch)
 						m.Post("/update", reqToken(), repo.UpdatePullRequest)
 						m.Get("/commits", repo.GetPullRequestCommits)
+						m.Get("/files", repo.GetPullRequestFiles)


just use context.ReferencesGitRepo(false), to get gitRepo injected into ctx

codecov-commenter · 2022-01-22T23:04:18Z

Codecov Report

❗ No coverage uploaded for pull request base (main@1ddfa59). Click here to learn what that means.
The diff coverage is 1.51%.

@@           Coverage Diff           @@
##             main   #18228   +/-   ##
=======================================
  Coverage        ?   46.02%           
=======================================
  Files           ?      838           
  Lines           ?    92791           
  Branches        ?        0           
=======================================
  Hits            ?    42704           
  Misses          ?    43303           
  Partials        ?     6784

Impacted Files	Coverage Δ
modules/convert/convert.go	`78.22% <0.00%> (ø)`
routers/api/v1/repo/pull.go	`25.68% <0.00%> (ø)`
routers/api/v1/api.go	`78.22% <100.00%> (ø)`

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 1ddfa59...30a9ed6. Read the comment docs.

lafriks · 2022-01-26T00:56:42Z

Would be nice if it would return structure same as in https://docs.github.com/en/rest/reference/pulls#list-pull-requests-files even if it would return only filenames so that it could be extended easier in the future

6543 · 2022-02-02T16:02:24Z

☝️

charlesmelby · 2022-04-10T09:44:17Z

Any progress on this? It looks like this is needed for several CI-related integrations.

anbraten · 2022-04-10T13:05:38Z

I haven't looked at this PR for a while. You are right its required for some CI systems that's the reason I started it 😉. My general plan was to create the most basic version for now just a list of paths of all changed files with a structure similar to the one of Github. This way it would be easily extendable in the future if needed.

d0x7 · 2022-08-13T04:02:52Z

Any update? Do you maybe wanna integrate the requested changes and if the team is willing to, it could be released in this state, and when needed expanded feature-wise? I think it's somewhat important for Woodpeeker, Drone and other CIs. Including myself - a bot I'm writing for a internal project would somewhat rely on this existing.

anbraten · 2022-09-25T21:13:56Z

seems like #21177 is winning the race 🏇🏾 😃

lunny · 2022-09-26T06:15:27Z

Please join the discussion there. :)

This adds an api endpoint `/files` to PRs that allows to get a list of changed files. built upon #18228, reviews there are included closes #654 Co-authored-by: Anton Bracke <[email protected]> Co-authored-by: 6543 <[email protected]> Co-authored-by: wxiaoguang <[email protected]>

anbraten added 3 commits January 10, 2022 17:20

add api endpoint to get changed files of a pr

5ff626e

Merge branch 'main' into pr-changed-files-api

fdf926f

update swagger definitions

0c807d1

lunny added the modifies/api This PR adds API routes or modifies them label Jan 11, 2022

GiteaBot added the lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. label Jan 11, 2022

Gusted reviewed Jan 11, 2022

View reviewed changes

6543 added the type/feature Completely new functionality. Can only be merged if feature freeze is not active. label Jan 14, 2022

6543 reviewed Jan 14, 2022

View reviewed changes

anbraten mentioned this pull request Jan 16, 2022

Support ChangedFiles for Github & Gitlab PRs and Gitea pushes woodpecker-ci/woodpecker#697

Merged

2 tasks

zeripath changed the title ~~add api endpoint to get changed files of a pr~~ Add API endpoint to get changed files of a PR Jan 19, 2022

zeripath removed the type/feature Completely new functionality. Can only be merged if feature freeze is not active. label Jan 19, 2022

wxiaoguang added this to the 1.17.0 milestone Jan 20, 2022

Merge branch 'main' into pr-changed-files-api

30a9ed6

techknowlogick modified the milestones: 1.17.0, 1.18.0 Jun 8, 2022

qwerty287 mentioned this pull request Sep 15, 2022

Add API endpoint to get changed files of a PR #21177

Merged

anbraten closed this Sep 25, 2022

lunny removed this from the 1.18.0 milestone Sep 26, 2022

go-gitea locked and limited conversation to collaborators May 3, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add API endpoint to get changed files of a PR #18228

Add API endpoint to get changed files of a PR #18228

anbraten commented Jan 10, 2022

lunny commented Jan 11, 2022 •

edited

Loading

Gusted left a comment

Gusted Jan 11, 2022

Gusted Jan 11, 2022

Gusted Jan 11, 2022

Gusted Jan 11, 2022

Gusted Jan 11, 2022

Gusted Jan 11, 2022

6543 Jan 14, 2022

codecov-commenter commented Jan 22, 2022

lafriks commented Jan 26, 2022

6543 commented Feb 2, 2022

charlesmelby commented Apr 10, 2022

anbraten commented Apr 10, 2022

d0x7 commented Aug 13, 2022

anbraten commented Sep 25, 2022

lunny commented Sep 26, 2022

	ctx.ServerError("GetDiffRangeWithWhitespaceBehavior", err)
	ctx.ServerError("GetDiff", err)

		apiFile := convert.ToChangedFile(diff.Files[i])
		apiFiles = append(apiFiles, apiFile)

	apiFile := convert.ToChangedFile(diff.Files[i])
	apiFiles = append(apiFiles, apiFile)
	apiFiles = append(apiFiles, convert.ToChangedFile(diff.Files[i]))

Add API endpoint to get changed files of a PR #18228

Add API endpoint to get changed files of a PR #18228

Conversation

anbraten commented Jan 10, 2022

lunny commented Jan 11, 2022 • edited Loading

Gusted left a comment

Choose a reason for hiding this comment

Gusted Jan 11, 2022

Choose a reason for hiding this comment

Gusted Jan 11, 2022

Choose a reason for hiding this comment

Gusted Jan 11, 2022

Choose a reason for hiding this comment

Gusted Jan 11, 2022

Choose a reason for hiding this comment

Gusted Jan 11, 2022

Choose a reason for hiding this comment

Gusted Jan 11, 2022

Choose a reason for hiding this comment

6543 Jan 14, 2022

Choose a reason for hiding this comment

codecov-commenter commented Jan 22, 2022

Codecov Report

lafriks commented Jan 26, 2022

6543 commented Feb 2, 2022

charlesmelby commented Apr 10, 2022

anbraten commented Apr 10, 2022

d0x7 commented Aug 13, 2022

anbraten commented Sep 25, 2022

lunny commented Sep 26, 2022

lunny commented Jan 11, 2022 •

edited

Loading