Add ProjectsService #3718

JoannaaKL · 2025-09-17T14:36:16Z

First chunk to support new GitHub Projects API:

ListProjects for User
ListProjects for Org
GetProject for User
GetProject for Org
List Project Fields For Org

Additionally I added an integration test for the new api - it was helpful for me to run it agains the real API and check if all fields make sense. Lmk if you want to keep it!

alexandear · 2025-09-17T14:44:52Z

This PR won't be merged soon because the only one maintainer, @gmlewis, has lost access to the repo. See #3689 (comment)

So, let's wait when @google-admin grants access, and then proceed with adding the feature.

gmlewis

Thank you, @JoannaaKL!
One tiny tweak, please, and then LGTM.
Could you also please rename the PR from "Add projects" to "Add ProjectsService"?

github/projects.go

gmlewis · 2025-09-18T14:46:31Z

Also, please note that I still don't have write access to this repo, but figure I might as well attempt to catch up on my code reviews (which I usually allow the linter to process first... ah, well).

github/projects.go

JoannaaKL · 2025-09-19T12:40:39Z

Hey hey @gmlewis 👋 . I renamed the pr, there's one more tiny change I need to do to this pr before it's ready - update the pagination parameters. I hope you get your access back soon!

gmlewis · 2025-09-19T12:43:20Z

Hey hey @gmlewis 👋 . I renamed the pr, there's one more tiny change I need to do to this pr before it's ready - update the pagination parameters. I hope you get your access back soon!

Thank you, @JoannaaKL! Me too!!! ❤️

alexandear

req.Header.Set("Accept", mediaTypeProjectsPreview) should be removed.

codecov · 2025-09-22T19:27:56Z

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 91.15%. Comparing base (b7f068f) to head (a11a2fa).
⚠️ Report is 1 commits behind head on master.

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #3718      +/-   ##
==========================================
+ Coverage   91.11%   91.15%   +0.03%     
==========================================
  Files         187      188       +1     
  Lines       16702    16776      +74     
==========================================
+ Hits        15218    15292      +74     
  Misses       1296     1296              
  Partials      188      188

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

JoannaaKL · 2025-09-22T20:01:54Z

One todo item I have to follow up on tomorrow is making sure the pagination is correct

gmlewis · 2025-09-22T22:18:49Z

@JoannaaKL - could you please also update the branch with the latest changes from master while you are at it?
Thank you!

JoannaaKL · 2025-09-23T10:21:53Z

Hi @gmlewis I ran ./script/metadata.sh update-openapi and then ./script/generate - linter was failing because generated files were outdated. Lmk if you're ok with leaving these changes in this pr.

gmlewis

Thank you, @JoannaaKL!
LGTM.
Awaiting second LGTM+Approval from any other contributor to this repo before merging.

cc: @stevehipwell - @alexandear

github/projects.go

openapi_operations.yaml

gmlewis · 2025-09-25T14:15:40Z

@JoannaaKL - now that #3735 is merged, could you please merge master into this PR and revert all changes to openapi_operations.yaml? Thank you!

Co-authored-by: Oleksandr Redko <[email protected]>

github/projects.go

MattBabbage

Looks great + great feedback from others

alexandear · 2025-09-26T10:50:25Z

github/projects.go

+// GitHub API docs: https://docs.github.com/rest/projects/projects#get-project-for-organization
+//
+//meta:operation GET /orgs/{org}/projectsV2/{project_number}
+func (s *ProjectsService) GetProjectForOrg(ctx context.Context, org string, projectNumber int64) (*ProjectV2, *Response, error) {


Perhaps the type of projectNumber should be the same as the type of Number in the ProjectV2 structure:

Suggested change

func (s *ProjectsService) GetProjectForOrg(ctx context.Context, org string, projectNumber int64) (*ProjectV2, *Response, error) {

func (s *ProjectsService) GetProjectForOrg(ctx context.Context, org string, projectNumber int) (*ProjectV2, *Response, error) {

Great catch, @alexandear! This made me curious if the Number property ever might be as huge as an "id" and as far as I can tell, that is not the case, so I do NOT think both should be int64, but instead both should be int as you have suggested.

Thoughts, @JoannaaKL?

In the GH API project's ID is a number type and project_number is an integer type. It can hold int64 actually. number type can hold any number so yes, I think it should be equal to int 64 as it is now. So I don't think they should be set to int.

In the GH API project's ID is a number type and project_number is an integer type. It can hold int64 actually. number type can hold any number so yes, I think it should be equal to int 64 as it is now. So I don't think they should be set to int.

I think we may have a disconnect here.

The ProjectV2 struct has these fields (among others):

ID *int64 `json:"id,omitempty"` Number *int `json:"number,omitempty"`

IDs in this repo are (now) always of type int64 because we empirically discovered IDs from GitHub many years ago that did not fit within an int, so that is our policy moving forward.

Other integers in this repo that we never expect to overflow an int remain an int because that is the natural and familiar integer type for Go programmers.

What we are saying here is that we believe the ProjectV2.Number field should remain an int and on line 136 above you currently have projectNumber int64 which does not match its corresponding field in the struct.

Therefore, we would like you to change projectNumber int64 to projectNumber int to make the two of them match.

Does this make sense, @JoannaaKL?

I was talking about types described in the doc https://docs.github.com/en/rest/projects/fields?apiVersion=2022-11-28
projectNumber there is described as integer and that type can hold int64 link (correct me if I am wrong).

JoannaaKL · 2025-09-26T12:21:04Z

@gmlewis @alexandear I have some feedback regarding the pr review process - it would be much more productive if you did one round of review and ask for changes instead of this ping-pong we are doing. In every review you leave more comments and questions, after they are resolved you add another one. I understand that you are busy and it's not easy to find the time but I don't think it is the best use of our time.

gmlewis · 2025-09-26T12:28:26Z

@gmlewis @alexandear I have some feedback regarding the pr review process - it would be much more productive if you did one round of review and ask for changes instead of this ping-pong we are doing. In every review you leave more comments and questions, after they are resolved you add another one. I understand that you are busy and it's not easy to find the time but I don't think it is the best use of our time.

Unfortunately, @JoannaaKL, after working on Open Source projects for 9 years now, this is the nature of the process.

Everyone here is volunteering their own time apart from whatever other demands (like work, family, etc.) are upon them.

And there are policies that have been put in place for maintenance of open source projects like this one where we require two approvals for non-trivial changes (such as this one) before merging.

As witnessed by the history of this repo, I am far from perfect in my code reviews and others invariably find things that I have missed.

So I apologize for the inconvenience we are causing you, but there is not a better process or solution that I am aware of.

Also, we obviously have a significant disconnect above, as we are asking for what appears to me to be a simple change, but you are talking about the definition of numbers in swagger documentation which to me seems to be irrelevant. I believe that if we can resolve this issue, then this PR will be ready for merging, as we are so close.

gmlewis

Thank you, @JoannaaKL!
LGTM.

@alexandear - do you approve this PR now?

alexandear · 2025-10-02T15:18:13Z

test/integration/projects_test.go

+	// List projects for org; pick the first available project we can read.
+	projects, _, err := client.Projects.ListProjectsForOrg(ctx, org, opts)
+	if err != nil {
+		// If listing itself fails, abort this test.


This comment is redundant and can be removed without worsing readability and maintanability.

alexandear · 2025-10-02T15:18:52Z

test/integration/projects_test.go

+
+	proj, _, err := client.Projects.GetProjectForUser(ctx, user, *project.Number)
+	if err != nil {
+		// can't fetch specific project; treat as fatal


Why do we need this comment?

test/integration/projects_test.go

alexandear · 2025-10-02T15:26:12Z

test/integration/projects_test.go

+// Integration tests for Projects V2 endpoints defined in github/projects.go.
+//
+// These tests are intentionally defensive. They only require minimal
+// environment variables identifying a target org and user. Project numbers are
+// discovered dynamically by first listing projects and selecting one. For item
+// CRUD operations, the test creates a temporary repository & issue (where
+// possible) and adds/removes that issue as a project item. If prerequisites
+// (auth, env vars, permissions, presence of at least one project) are missing,
+// the relevant sub-test is skipped so other integration tests can still run.
+//
+// Required / optional environment variables:
+//   GITHUB_AUTH_TOKEN                  (required for any of these tests to run)
+//   GITHUB_TEST_ORG                    (org login; required for org project tests)
+//   GITHUB_TEST_USER                   (user login; required for user project tests)
+//   GITHUB_TEST_REPO                   (repo name)


I propose splitting the comment into separate comments for TestProjectsV2_Org and TestProjectsV2_User. We don't need to mention GITHUB_AUTH_TOKEN since it's documented in the README:

go-github/test/README.md

Line 32 in b7f068f

GITHUB_AUTH_TOKEN=XXX go test -v -tags=integration ./integration

.

Co-authored-by: Oleksandr Redko <[email protected]>

JoannaaKL added 2 commits September 17, 2025 14:31

Add support for projects V2

775f0f6

Add test

74712bd

JoannaaKL marked this pull request as ready for review September 17, 2025 16:10

gmlewis requested changes Sep 18, 2025

View reviewed changes

github/projects.go Outdated Show resolved Hide resolved

alexandear reviewed Sep 18, 2025

View reviewed changes

JoannaaKL changed the title ~~Add projects~~ Add ProjectsService Sep 19, 2025

Address feedback

3a8cf0c

JoannaaKL force-pushed the add-projects branch from 73aeb75 to 3a8cf0c Compare September 22, 2025 13:52

alexandear suggested changes Sep 22, 2025

View reviewed changes

JoannaaKL added 2 commits September 22, 2025 20:42

Merge branch 'master' into add-projects

b754eeb

Remove header and rename functions

e93ed36

JoannaaKL requested a review from gmlewis September 22, 2025 19:08

Update comments

99852cf

Copyright update

7664215

gmlewis added the NeedsReview PR is awaiting a review before merging. label Sep 22, 2025

JoannaaKL added 6 commits September 23, 2025 09:11

Merge branch 'master' into add-projects

89016d9

Update comments

2b94765

Generate docs

a4b0a56

Add list projects option

696102b

Generate openapi docs

469e03a

Results of generate

2dbe5b4

Merge branch 'master' into add-projects

478ef5a

gmlewis approved these changes Sep 24, 2025

View reviewed changes

JoannaaKL added 2 commits September 24, 2025 16:56

Merge branch 'master' into add-projects

9d6bf4b

Merge branch 'master' into add-projects

9f4c89d

alexandear suggested changes Sep 25, 2025

View reviewed changes

github/projects.go Outdated Show resolved Hide resolved

github/projects.go Outdated Show resolved Hide resolved

openapi_operations.yaml Show resolved Hide resolved

Merge branch 'master' into add-projects

3dd69be

gmlewis mentioned this pull request Sep 25, 2025

Update openapi_operations.yaml #3735

Merged

Update github/projects.go

275f930

Co-authored-by: Oleksandr Redko <[email protected]>

alexandear reviewed Sep 25, 2025

View reviewed changes

github/projects.go Show resolved Hide resolved

JoannaaKL added 4 commits September 26, 2025 10:07

Sync openapi_operations.yaml

aca15c0

Merge branch 'master' into add-projects

8ee5bc4

Address pr comments

0ca0521

Add integration test and minor tweaks

fb6e7a8

JoannaaKL requested review from alexandear and gmlewis September 26, 2025 09:05

MattBabbage approved these changes Sep 26, 2025

View reviewed changes

alexandear reviewed Sep 26, 2025

View reviewed changes

gmlewis added the waiting for reply label Sep 27, 2025

JoannaaKL added 2 commits October 1, 2025 14:37

Merge branch 'master' into add-projects

6d0637f

.

5cc797e

JoannaaKL force-pushed the add-projects branch from 2f2c53a to 5cc797e Compare October 1, 2025 12:45

gmlewis approved these changes Oct 1, 2025

View reviewed changes

alexandear approved these changes Oct 2, 2025

View reviewed changes

JoannaaKL and others added 5 commits October 3, 2025 05:22

Update test/integration/projects_test.go

c6cfe31

Co-authored-by: Oleksandr Redko <[email protected]>

Update test/integration/projects_test.go

c9feeb2

Co-authored-by: Oleksandr Redko <[email protected]>

Update test/integration/projects_test.go

8c985a9

Co-authored-by: Oleksandr Redko <[email protected]>

Update test/integration/projects_test.go

1625c61

Co-authored-by: Oleksandr Redko <[email protected]>

Merge branch 'master' into add-projects

a11a2fa

	func (s ProjectsService) GetProjectForOrg(ctx context.Context, org string, projectNumber int64) (ProjectV2, *Response, error) {
	func (s ProjectsService) GetProjectForOrg(ctx context.Context, org string, projectNumber int) (ProjectV2, *Response, error) {

Add ProjectsService #3718

Are you sure you want to change the base?

Add ProjectsService #3718

Uh oh!

Conversation

JoannaaKL commented Sep 17, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

alexandear commented Sep 17, 2025

Uh oh!

gmlewis left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

gmlewis commented Sep 18, 2025

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

JoannaaKL commented Sep 19, 2025

Uh oh!

gmlewis commented Sep 19, 2025

Uh oh!

alexandear left a comment

Choose a reason for hiding this comment

Uh oh!

codecov bot commented Sep 22, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

JoannaaKL commented Sep 22, 2025

Uh oh!

gmlewis commented Sep 22, 2025

Uh oh!

JoannaaKL commented Sep 23, 2025

Uh oh!

gmlewis left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

gmlewis commented Sep 25, 2025

Uh oh!

Uh oh!

MattBabbage left a comment

Choose a reason for hiding this comment

Uh oh!

alexandear Sep 26, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

gmlewis Sep 26, 2025

Choose a reason for hiding this comment

Uh oh!

JoannaaKL Sep 26, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

gmlewis Sep 26, 2025

Choose a reason for hiding this comment

Uh oh!

JoannaaKL Sep 26, 2025

Choose a reason for hiding this comment

Uh oh!

JoannaaKL commented Sep 26, 2025

Uh oh!

gmlewis commented Sep 26, 2025

Uh oh!

gmlewis left a comment

Choose a reason for hiding this comment

Uh oh!

alexandear Oct 2, 2025

Choose a reason for hiding this comment

Uh oh!

alexandear Oct 2, 2025

JoannaaKL commented Sep 17, 2025 •

edited

Loading

codecov bot commented Sep 22, 2025 •

edited

Loading

alexandear Sep 26, 2025 •

edited

Loading

JoannaaKL Sep 26, 2025 •

edited

Loading