Skip to content

Conversation

@alkak95
Copy link
Contributor

@alkak95 alkak95 commented Oct 28, 2025

BREAKING CHANGE: RepositoriesService.CreateStatus now takes value for status, not pointer.

Relates to: #3644

Description: fixes part of issue Refactor codebase to use value parameters instead of pointers where appropriate #3644

Fix: To improve API design consistency and safety as per issue, removed unnecessary pointer in the file repo_statuses.go

Tested by running go test ./... command.

@alkak95
Copy link
Contributor Author

alkak95 commented Oct 28, 2025

@gmlewis I have made a sample change , If it looks good, I will go ahead and make changes in other files as well.
This change is made because it's a POST API and it should have some body to process the request, hence removed the pointer from func signature.

@gmlewis gmlewis changed the title Remove pointer from required field of CreateStatus API refactor!: Remove pointer from required field of CreateStatus API Oct 28, 2025
@codecov
Copy link

codecov bot commented Oct 28, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 92.17%. Comparing base (22f7fe3) to head (98357c9).
⚠️ Report is 1 commits behind head on master.

Additional details and impacted files
@@           Coverage Diff           @@
##           master    #3794   +/-   ##
=======================================
  Coverage   92.17%   92.17%           
=======================================
  Files         191      191           
  Lines       13690    13690           
=======================================
  Hits        12619    12619           
  Misses        883      883           
  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.

@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 28, 2025
Copy link
Collaborator

@gmlewis gmlewis left a comment

Choose a reason for hiding this comment

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

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

cc: @stevehipwell - @alexandear - @zyfy29

Copy link
Contributor

@zyfy29 zyfy29 left a comment

Choose a reason for hiding this comment

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

LGTM

@gmlewis
Copy link
Collaborator

gmlewis commented Oct 28, 2025

Thank you, @zyfy29!
Merging.

@gmlewis gmlewis removed the NeedsReview PR is awaiting a review before merging. label Oct 28, 2025
@gmlewis gmlewis merged commit 53d0e6a into google:master Oct 28, 2025
7 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