Skip to content

refactor!: Pass workflow settings by value - #4679

Merged
gmlewis merged 2 commits into
google:masterfrom
55408suga:refactor/workflows-permissions-request
Oct 28, 2026
Merged

gmlewis merged 2 commits into
google:masterfrom
55408suga:refactor/workflows-permissions-request

Conversation

@55408suga

@55408suga 55408suga commented Oct 27, 2026 •

Copy link
Copy Markdown
Contributor

Rename WorkflowsPermissionsOpt to WorkflowsPermissionsRequest and pass it by value in the repository, organization, and enterprise fork PR workflow settings methods.

Preserve existing JSON behavior, including the distinction between omitted optional booleans and explicit false. Update tests and generated accessors, and remove the obsolete linter exception.

Updates #3644.

BREAKING CHANGE: Callers must use WorkflowsPermissionsRequest values instead of *WorkflowsPermissionsOpt.

Validation:

  • script/generate.sh
  • script/fmt.sh
  • script/test.sh (all modules, with race detection)
  • script/lint.sh
  • go test -tags=integration -run='^$' ./test/integration (compile only)

AI assistance: An AI coding agent (Codex) drafted the code, tests, and this description. I reviewed the diff and ran the validation steps above locally.

Rename the shared workflow permissions request and pass it by value in
the repository, organization, and enterprise settings methods. Preserve
boolean JSON behavior and update generated accessors and tests.

BREAKING CHANGE: WorkflowsPermissionsOpt is renamed to
WorkflowsPermissionsRequest. The three UpdatePrivateRepoForkPRWorkflowSettings
methods now take the request by value instead of by pointer.

Updates google#3644
@55408suga
55408suga marked this pull request as ready for review October 27, 2026 13:14
@gmlewis gmlewis added NeedsReview PR is awaiting a review before merging. Breaking API Change PR will require a bump to the major version num in next release. Look here to see the change(s). labels Oct 27, 2026
@codecov

codecov Bot commented Oct 27, 2026 •

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 98.63%. Comparing base (6eccc42) to head (260274e).
⚠️ Report is 1 commits behind head on master.

Additional details and impacted files
@@           Coverage Diff           @@
##           master    #4679   +/-   ##
=======================================
  Coverage   98.63%   98.63%           
=======================================
  Files         201      201           
  Lines       18951    18951           
=======================================
  Hits        18693    18693           
  Misses        258      258           

☔ View full report in Codecov by Harness.
📢 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.

Comment thread github/actions_workflows_test.go Outdated
Co-authored-by: Dhananjay Mishra <technicaldmcontact@gmail.com>

@Not-Dhananjay-Mishra Not-Dhananjay-Mishra left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@gmlewis gmlewis removed the NeedsReview PR is awaiting a review before merging. label Oct 28, 2026
@gmlewis

gmlewis commented Oct 28, 2026

Copy link
Copy Markdown
Collaborator

Thank you, @Not-Dhananjay-Mishra!
Merging.

@gmlewis
gmlewis merged commit 2ca80c0 into google:master Oct 28, 2026
15 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Breaking API Change PR will require a bump to the major version num in next release. Look here to see the change(s).

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants