fix!: Add ListCursorOptions to Enterprise and Org code security configs

[maishivamhoo123]

This PR updates ListEnterpriseCodeSecurityConfigurationOptions and ListCodeSecurityConfigurationRepositoriesOptions to embed ListCursorOptions.

Changes:

• Replaced manual pagination fields (Before, After, PerPage) with ListCursorOptions embedding.
  Reason:
  These endpoints support cursor-based pagination. Explicitly embedding ListCursorOptions ensures the library generates the correct Iter methods, preventing infinite loops and aligning with the GitHub API.

Issue Related #3976

[alexandear]

@gmlewis maybe it is better to simply extend the gen-iterators.go to support generating iterator methods from the ListEnterpriseCodeSecurityConfigurationOptions?

[alexandear]

I don't like this kind of breaking change.

[gmlewis]

I'm not sure I understand, @alexandear - the point of using ListOptions and ListCursorOptions is to standardize the pagination handling. Since this endpoint uses cursor pagination, it makes sense to me to embed ListCursorOptions like we do with the other endpoints that support cursor pagination.
The next release is already going to have a large number of breaking API changes, mostly related to pagination, so I think it is fine to include this one.

[alexandear]

ListCursorOptions contains more fields than the GitHub API expects. From a user's perspective, it's better to have a simpler ListEnterpriseCodeSecurityConfigurationOptions with only three fields: before, after, and per_page.

ListCursorOptions:

go-github/github/github.go

Lines 267 to 290 in 1732098

	typeListCursorOptionsstruct {
	// For paginated result sets, page of results to retrieve.
	Pagestring`url:"page,omitempty"`
	// For paginated result sets, the number of results to include per page.
	PerPageint`url:"per_page,omitempty"`
	// For paginated result sets, the number of results per page (max 100), starting from the first matching result.
	// This parameter must not be used in combination with last.
	Firstint`url:"first,omitempty"`
	// For paginated result sets, the number of results per page (max 100), starting from the last matching result.
	// This parameter must not be used in combination with first.
	Lastint`url:"last,omitempty"`
	// A cursor, as given in the Link header. If specified, the query only searches for events after this cursor.
	Afterstring`url:"after,omitempty"`
	// A cursor, as given in the Link header. If specified, the query only searches for events before this cursor.
	Beforestring`url:"before,omitempty"`
	// A cursor, as given in the Link header. If specified, the query continues the search using this cursor.
	Cursorstring`url:"cursor,omitempty"`
	}

[alexandear]

Anyway, if we decide to use ListCursorOptions, let's simply inline it instead of ListEnterpriseCodeSecurityConfigurationOptions:

func (s*EnterpriseService) ListCodeSecurityConfigurations(ctx context.Context, enterprisestring, opts*ListCursorOptions) ([]*CodeSecurityConfiguration, *Response, error) {

[maishivamhoo123]

@alexandear Sir Thank you for the suggestion.

I feel we should keep the wrapper struct as it is, rather than inlining ListCursorOptions.

The reason is that github/gen-iterators.go relies on the wrapper to generate the iterator. Specifically, the hasOptions function iterates over the Embeds list to find a match:
https://github.com/google/go-github/blob/master/github/gen-iterators.go#L177

Since ListCursorOptions defines fields directly and does not embed itself, hasOptions returns false if we use it directly in the function signature. This causes the generator to skip creating the Iter method for this endpoint entirely.

To support inlining, we would likely need to refactor the generator logic itself. Given this, and the benefit of future extensibility, I believe keeping the wrapper struct is the safer approach.

Please let me know if I have misunderstood the logic or if there is a better way to handle this.

[alexandear]

To support inlining, we would likely need to refactor the generator logic itself

You're correct.

Since we had already handled direct ListOptions, I added support for ListCursorOptions in #4007 for consistency.

[gmlewis]

Thank you, @alexandear! I now see what you mean. 😂
Sounds good to me.

[maishivamhoo123]

@alexandear Sir, I understand now. Instead of forcing a wrapper struct, you updated the generator to recursively check if the struct contains the After field. That makes sense!

@gmlewis@alexandear Sir I have a small doubt as there are multiple recursive functions now checking these structs. Since they might be processing the same embedded structs repeatedly, would it be beneficial to apply memoization (Dynamic Programming)?

We could cache the result for each struct type. If a struct is already checked, we can return the result immediately from the cache. This might decrease the generation time.

Thank you, Sir!

[gmlewis]

We could cache the result for each struct type. If a struct is already checked, we can return the result immediately from the cache. This might decrease the generation time.

You are welcome to experiment and see if you can improve the runtime of all these linter checks.
If you are successful, please open a separate PR with your improvements.

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 93.54%. Comparing base (ab5ad47) to head (72b1706).
⚠️ Report is 8 commits behind head on master.

Additional details and impacted files

@@ Coverage Diff @@## master #3997 +/- ##
==========================================
+ Coverage 93.52% 93.54% +0.01% 
==========================================
Files 207 207 Lines 17597 17648 +51 ==========================================
+ Hits 16458 16509 +51 
Misses 938 938 Partials 201 201 

☔ 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.

[gmlewis]

@maishivamhoo123 and @alexandear - are we agreed that this PR can be closed due to #4007?

[alexandear]

@maishivamhoo123 and @alexandear - are we agreed that this PR can be closed due to #4007?

Yes. This PR is not needed anymore.

Now we have EnterpriseService.ListCodeSecurityConfigurationsIter and OrganizationsService.ListCodeSecurityConfigurationsIter.

[gmlewis]

Closing as obsolete.