Skip to content

feat: Add artifact and log retention period API support #3664

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 6 commits into from
Aug 11, 2025

Conversation

zyfy29
Copy link
Contributor

@zyfy29 zyfy29 commented Aug 11, 2025

1 of 4 apis in #3660

The "Setting artifact and log retention periods" api for each of repos, organizations and enterprises.

@Copilot Copilot AI review requested due to automatic review settings August 11, 2025 08:25
Copy link

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR adds GitHub API support for setting artifact and log retention periods across repositories, organizations, and enterprises. This is part 1 of 4 APIs mentioned in issue #3660.

  • Introduces new types ArtifactPeriod and ArtifactPeriodOpt for handling retention period data
  • Adds GET and PUT methods for each scope (repository, organization, enterprise) to retrieve and update retention settings
  • Includes comprehensive test coverage for all new API methods

Reviewed Changes

Copilot reviewed 10 out of 10 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
github/actions_artifacts.go Defines new types ArtifactPeriod and ArtifactPeriodOpt for retention period data structures
github/repos_actions_permissions.go Adds repository-level GET/PUT methods for artifact and log retention periods
github/actions_permissions_orgs.go Adds organization-level GET/PUT methods for artifact and log retention periods
github/actions_permissions_enterprise.go Adds enterprise-level GET/PUT methods for artifact and log retention periods
github/repos_actions_permissions_test.go Test coverage for repository-level retention period methods
github/actions_permissions_orgs_test.go Test coverage for organization-level retention period methods
github/actions_permissions_enterprise_test.go Test coverage for enterprise-level retention period methods
github/github-accessors.go Generated getter methods for the new types
github/github-accessors_test.go Generated tests for the accessor methods
github/github-stringify_test.go Test for the String() method implementation

@zyfy29 zyfy29 changed the title feat: Add artifact and log retension period api support feat: Add artifact and log retention period api support Aug 11, 2025
@zyfy29
Copy link
Contributor Author

zyfy29 commented Aug 11, 2025

I got a little confused by the namespaces of the client. For example:

// GetActionsPermissions gets the GitHub Actions permissions policy for repositories and allowed actions in an organization.
//
// Deprecated: please use `client.Actions.GetActionsPermissions` instead.
//
// GitHub API docs: https://docs.github.com/rest/actions/permissions#get-github-actions-permissions-for-an-organization
//
//meta:operation GET /orgs/{org}/actions/permissions
func (s *OrganizationsService) GetActionsPermissions(ctx context.Context, org string) (*ActionsPermissions, *Response, error) {
s2 := (*ActionsService)(s)
return s2.GetActionsPermissions(ctx, org)
}

The "get (as well as edit) actions permissions" api for organizations is defined at ActionsService and existing code provides a alias at OrganizationsService. But other actions permissions methods have no aliases. So I didn't provide a alias for my implementation.

If I'm correct, I'll then open new prs to implement the rest of 3 apis.

Copy link

codecov bot commented Aug 11, 2025

Codecov Report

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

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #3664      +/-   ##
==========================================
+ Coverage   91.20%   91.23%   +0.03%     
==========================================
  Files         185      185              
  Lines       16360    16421      +61     
==========================================
+ Hits        14921    14982      +61     
  Misses       1254     1254              
  Partials      185      185              

☔ 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 changed the title feat: Add artifact and log retention period api support feat: Add artifact and log retention period API support Aug 11, 2025
@gmlewis gmlewis added the NeedsReview PR is awaiting a review before merging. label Aug 11, 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, @zyfy29!
Just a couple godoc comments needed, otherwise LGTM.
After providing those, we should be ready for a second LGTM+Approval from any other contributor to this repo before merging.

@zyfy29
Copy link
Contributor Author

zyfy29 commented Aug 11, 2025

Thank you for the review, fixed.

@zyfy29 zyfy29 requested a review from gmlewis August 11, 2025 12:59
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, @zyfy29!
LGTM.

Awaiting second LGTM+Approval from any other contributor to this repo before merging.

@alexandear or @stevehipwell - might one of you have time for a code review? Thank you!

Copy link
Contributor

@stevehipwell stevehipwell 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 gmlewis removed the NeedsReview PR is awaiting a review before merging. label Aug 11, 2025
@gmlewis
Copy link
Collaborator

gmlewis commented Aug 11, 2025

Thank you, @stevehipwell!
Merging.

@gmlewis gmlewis merged commit 84a0945 into google:master Aug 11, 2025
7 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants