Skip to content

Conversation

ericmort
Copy link

@ericmort ericmort commented Aug 30, 2025

BREAKING CHANGE: GetRepositoriesForCodeSecurityConfiguration now returns RepositoryAttachment instead of Repository.

Adds type RepositoryAttachment for GetRepositoriesForCodeSecurityConfiguration

Fixes unmarshal error in GetRepositoriesForCodeSecurityConfiguration, as described in #3217.

(I submitted the PR under my corporate GitHub user, but I could not get a proper google/CLA so using my personal user.)

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, @ericmort!
LGTM.

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

NOTES TO SELF:

  • Add to the description:
    BREAKING CHANGE: GetRepositoriesForCodeSecurityConfiguration now returns RepositoryAttachment instead of Repository.
  • Add Breaking API label
  • Change PR title:
    fix!: Return RepositoryAttachment from GetRepositoriesForCodeSecurityConfiguration

@gmlewis
Copy link
Collaborator

gmlewis commented Sep 18, 2025

Fixes: #3217.

Copy link

codecov bot commented Sep 22, 2025

Codecov Report

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

Additional details and impacted files
@@           Coverage Diff           @@
##           master    #3707   +/-   ##
=======================================
  Coverage   91.11%   91.11%           
=======================================
  Files         187      187           
  Lines       16684    16686    +2     
=======================================
+ Hits        15202    15204    +2     
  Misses       1295     1295           
  Partials      187      187           

☔ 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 Sep 22, 2025
@gmlewis gmlewis changed the title fix #3217 unmarshal error fix!: Return RepositoryAttachment from GetRepositoriesForCodeSecurityConfiguration Sep 22, 2025
@gmlewis
Copy link
Collaborator

gmlewis commented Sep 22, 2025

@ericmort - would you mind resolving the conflicts in this PR or would you like me to do that?

@ericmort
Copy link
Author

@gmlewis I resolved it now

@gmlewis
Copy link
Collaborator

gmlewis commented Sep 23, 2025

@gmlewis I resolved it now

@ericmort - could you please investigate the lint and test failures?

@gmlewis
Copy link
Collaborator

gmlewis commented Sep 23, 2025

Thank you, @ericmort!

cc: @stevehipwell

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). NeedsReview PR is awaiting a review before merging.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants