Skip to content
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

Fix issues where functions were using the same PR numbers for differe… #20

Merged
merged 1 commit into from
Jun 13, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
33 changes: 23 additions & 10 deletions internal/github/checkPendingCI.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@ import (
"context"
"fmt"
"log"
"strconv"
"strings"
"time"

Expand All @@ -28,16 +27,16 @@ func CheckForPendingCI(ctx context.Context, githubClient *github.Client, cfg *co
owner, repo := parts[0], parts[1]

// Fetch community PRs using the FindCommunityPRs function
communityPRs, err := FindCommunityPRs(cfg, teamMembers, githubClient)
communityPRs, err := FindCommunityPRs(cfg, teamMembers, githubClient, owner, repo, fullRepo.MinimumNumber)
if err != nil {
return nil, err
}

for _, pr := range communityPRs {
ctx, cancel := context.WithTimeout(ctx, 30*time.Second) // 30 seconds timeout for each request
defer cancel()
prctx, prcancel := context.WithTimeout(ctx, 30*time.Second) // 30 seconds timeout for each request
defer prcancel()
// Dynamic cutoff time based on the last commit to the PR
lastCommitTime, err := getLastCommitTime(ctx, githubClient, owner, repo, pr.GetNumber())
lastCommitTime, err := getLastCommitTime(prctx, githubClient, owner, repo, pr.GetNumber())
if err != nil {
log.Printf("Error retrieving last commit time for PR #%d in %s/%s: %v", pr.GetNumber(), owner, repo, err)
continue
Expand All @@ -49,18 +48,18 @@ func CheckForPendingCI(ctx context.Context, githubClient *github.Client, cfg *co
continue
}

hasCIRuns, err := checkCIStatus(ctx, githubClient, owner, repo, pr.GetNumber())
hasCIRuns, err := checkCIStatus(prctx, githubClient, owner, repo, pr.GetNumber())
if err != nil {
log.Printf("Error checking CI status for PR #%d in %s/%s: %v", pr.GetNumber(), owner, repo, err)
continue
}

teamMemberActivity, err := checkTeamMemberActivity(ctx, githubClient, owner, repo, pr.GetNumber(), teamMembers, lastCommitTime)
teamMemberActivity, err := checkTeamMemberActivity(prctx, githubClient, owner, repo, pr.GetNumber(), teamMembers, lastCommitTime)
if err != nil {
log.Printf("Error checking team member activity for PR #%d in %s/%s: %v", pr.GetNumber(), owner, repo, err)
continue // or handle the error as needed
}
if !hasCIRuns || !teamMemberActivity {
if !hasCIRuns && !teamMemberActivity {
log.Printf("PR #%d in %s/%s by %s is ready for CI since %v but no CI actions have started yet, or it requires re-approval.", pr.GetNumber(), owner, repo, pr.User.GetLogin(), pr.CreatedAt)
pendingPRs = append(pendingPRs, pr.GetHTMLURL())
}
Expand Down Expand Up @@ -92,10 +91,24 @@ func getLastCommitTime(ctx context.Context, client *github.Client, owner, repo s
}

func checkCIStatus(ctx context.Context, client *github.Client, owner, repo string, prNumber int) (bool, error) {
checks, _, err := client.Checks.ListCheckRunsForRef(ctx, owner, repo, strconv.Itoa(prNumber), &github.ListCheckRunsOptions{})
pr, _, err := client.PullRequests.Get(ctx, owner, repo, prNumber)
if err != nil {
return false, err
return false, fmt.Errorf("failed to fetch pull request #%d: %w", prNumber, err)
}
// Obtaining CI runs for the most recent commit
commitSHA := pr.GetHead().GetSHA()

checks, _, err := client.Checks.ListCheckRunsForRef(ctx, owner, repo, commitSHA, &github.ListCheckRunsOptions{})
if err != nil {
return false, fmt.Errorf("failed to fetch check runs for ref %s: %w", commitSHA, err)
}
// Potentially add logic later for checking for passing CI
/*
for _, checkRun := range checks.CheckRuns {
if checkRun.GetStatus() != "completed" || checkRun.GetConclusion() != "success" {
return false, nil // There are check runs that are not completed or not successful
}
*/
return checks.GetTotal() > 0, nil
}

Expand Down
43 changes: 27 additions & 16 deletions internal/github/checkStalePRs.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import (
"context"
"fmt"
"log"
"strings"
"time"

"github.com/google/go-github/v60/github"
Expand All @@ -14,25 +15,35 @@ import (
// CheckStalePRs will return a list of PR URLs that have not been updated in the last 7 days by internal team members.
func CheckStalePRs(ctx context.Context, githubClient *github.Client, cfg *config.Config) ([]string, error) {
var stalePRUrls []string
cutoffDate := time.Now().Add(7 * 24 * time.Hour) // 7 days ago
cutoffDate := time.Now().Add(-7 * 24 * time.Hour) // 7 days ago
teamMembers, err := GetTeamMemberList(githubClient, cfg.InternalTeam)
if err != nil {
return nil, err
}
communityPRs, err := FindCommunityPRs(cfg, teamMembers, githubClient)
if err != nil {
return nil, err
}

for _, pr := range communityPRs {
repoName := pr.GetBase().GetRepo().GetFullName() // Get the full name of the repository
stale, err := isStale(ctx, githubClient, pr, teamMembers, cutoffDate) // Handle both returned values
for _, fullRepo := range cfg.CheckRepos {
log.Println("Checking repository:", fullRepo.Name)
parts := strings.Split(fullRepo.Name, "/")
if len(parts) != 2 {
return nil, fmt.Errorf("invalid repository name - must contain owner and repository: %s", fullRepo.Name)
}
owner, repo := parts[0], parts[1]

communityPRs, err := FindCommunityPRs(cfg, teamMembers, githubClient, owner, repo, fullRepo.MinimumNumber)
if err != nil {
log.Printf("Error checking if PR in repo %s is stale: %v", repoName, err)
continue // Skip this PR or handle the error appropriately
return nil, err
}
if stale {
stalePRUrls = append(stalePRUrls, pr.GetHTMLURL()) // Append if PR is confirmed stale

for _, pr := range communityPRs {
repoName := pr.GetBase().GetRepo().GetFullName() // Get the full name of the repository
stale, err := isStale(ctx, githubClient, pr, teamMembers, cutoffDate) // Handle both returned values
if err != nil {
log.Printf("Error checking if PR in repo %s is stale: %v", repoName, err)
continue // Skip this PR or handle the error appropriately
}
if stale {
stalePRUrls = append(stalePRUrls, pr.GetHTMLURL()) // Append if PR is confirmed stale
}
}
}
return stalePRUrls, nil
Expand All @@ -43,9 +54,9 @@ func isStale(ctx context.Context, githubClient *github.Client, pr *github.PullRe
listOptions := &github.ListOptions{PerPage: 100}
for {
// Create a context for each request
ctx, cancel := context.WithTimeout(ctx, 30*time.Second) // 30 seconds timeout for each request
defer cancel()
events, resp, err := githubClient.Issues.ListIssueTimeline(ctx, pr.Base.Repo.Owner.GetLogin(), pr.Base.Repo.GetName(), pr.GetNumber(), listOptions)
staleCtx, staleCancel := context.WithTimeout(ctx, 30*time.Second) // 30 seconds timeout for each request
defer staleCancel()
events, resp, err := githubClient.Issues.ListIssueTimeline(staleCtx, pr.Base.Repo.Owner.GetLogin(), pr.Base.Repo.GetName(), pr.GetNumber(), listOptions)
if err != nil {
return false, fmt.Errorf("failed to get timeline for PR #%d of repo %s: %w", pr.GetNumber(), pr.Base.Repo.GetName(), err)
}
Expand All @@ -57,7 +68,7 @@ func isStale(ctx context.Context, githubClient *github.Client, pr *github.PullRe
return false, nil
}
}
cancel() // Clean up the context at the end of the loop iteration
staleCancel() // Clean up the context at the end of the loop iteration
if resp.NextPage == 0 {
break
}
Expand Down
47 changes: 18 additions & 29 deletions internal/github/communityPRs.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,16 +3,14 @@ package github
import (
"context"
"fmt"
"log"
"strings"

"github.com/google/go-github/v60/github"

"github.com/chia-network/github-bot/internal/config"
)

// FindCommunityPRs obtains PRs based on provided filters
func FindCommunityPRs(cfg *config.Config, teamMembers map[string]bool, githubClient *github.Client) ([]*github.PullRequest, error) {
func FindCommunityPRs(cfg *config.Config, teamMembers map[string]bool, githubClient *github.Client, owner string, repo string, minimumNumber int) ([]*github.PullRequest, error) {
var finalPRs []*github.PullRequest
opts := &github.PullRequestListOptions{
State: "open",
Expand All @@ -24,38 +22,29 @@ func FindCommunityPRs(cfg *config.Config, teamMembers map[string]bool, githubCli
},
}

for _, fullRepo := range cfg.CheckRepos {
log.Println("Checking repository:", fullRepo.Name)
parts := strings.Split(fullRepo.Name, "/")
if len(parts) != 2 {
return nil, fmt.Errorf("invalid repository name - must contain owner and repository: %s", fullRepo.Name)
for {
pullRequests, resp, err := githubClient.PullRequests.List(context.Background(), owner, repo, opts)
if err != nil {
return nil, fmt.Errorf("error listing pull requests for %s/%s: %w", owner, repo, err)
}
owner, repo := parts[0], parts[1]

for {
pullRequests, resp, err := githubClient.PullRequests.List(context.TODO(), owner, repo, opts)
if err != nil {
return nil, fmt.Errorf("error listing pull requests for %s/%s: %w", owner, repo, err)
for _, pullRequest := range pullRequests {
if *pullRequest.Number < minimumNumber {
break
}

for _, pullRequest := range pullRequests {
if *pullRequest.Number < fullRepo.MinimumNumber {
break
}
if *pullRequest.Draft {
continue
}
user := *pullRequest.User.Login
if !teamMembers[user] && !cfg.SkipUsersMap[user] {
finalPRs = append(finalPRs, pullRequest)
}
if *pullRequest.Draft {
continue
}

if resp.NextPage == 0 {
break // Exit the loop if there are no more pages
user := *pullRequest.User.Login
if !teamMembers[user] && !cfg.SkipUsersMap[user] {
finalPRs = append(finalPRs, pullRequest)
}
opts.Page = resp.NextPage // Set next page number
}

if resp.NextPage == 0 {
break // Exit the loop if there are no more pages
}
opts.Page = resp.NextPage // Set next page number
}
return finalPRs, nil
}
70 changes: 40 additions & 30 deletions internal/label/pullrequests.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import (
"context"
"fmt"
"log"
"strings"

"github.com/google/go-github/v60/github"

Expand All @@ -18,43 +19,52 @@ func PullRequests(githubClient *github.Client, cfg *config.Config) error {
return fmt.Errorf("error getting team members: %w", err) // Properly handle and return error if team member list fetch fails
}

pullRequests, err := github2.FindCommunityPRs(cfg, teamMembers, githubClient)
if err != nil {
return fmt.Errorf("error finding community PRs: %w", err) // Handle error from finding community PRs
}

for _, pullRequest := range pullRequests {
user := *pullRequest.User.Login
for _, fullRepo := range cfg.CheckRepos {
log.Println("Checking repository:", fullRepo.Name)
parts := strings.Split(fullRepo.Name, "/")
if len(parts) != 2 {
log.Printf("invalid repository name - must contain owner and repository: %s", fullRepo.Name)
continue
}
owner, repo := parts[0], parts[1]

var label string
if teamMembers[user] {
label = cfg.LabelInternal
} else {
label = cfg.LabelExternal
pullRequests, err := github2.FindCommunityPRs(cfg, teamMembers, githubClient, owner, repo, fullRepo.MinimumNumber)
if err != nil {
return fmt.Errorf("error finding community PRs: %w", err) // Handle error from finding community PRs
}
if label != "" {
log.Printf("Pull Request %d by %s will be labeled %s\n", *pullRequest.Number, user, label)
hasLabel := false
for _, existingLabel := range pullRequest.Labels {
if *existingLabel.Name == label {
log.Println("Already labeled, skipping...")
hasLabel = true
break
}
}

if !hasLabel {
allLabels := []string{label}
for _, labelP := range pullRequest.Labels {
allLabels = append(allLabels, *labelP.Name)
for _, pullRequest := range pullRequests {
user := *pullRequest.User.Login

var label string
if teamMembers[user] {
label = cfg.LabelInternal
} else {
label = cfg.LabelExternal
}
if label != "" {
log.Printf("Pull Request %d by %s will be labeled %s\n", *pullRequest.Number, user, label)
hasLabel := false
for _, existingLabel := range pullRequest.Labels {
if *existingLabel.Name == label {
log.Println("Already labeled, skipping...")
hasLabel = true
break
}
}
_, _, err := githubClient.Issues.AddLabelsToIssue(context.TODO(), *pullRequest.Base.Repo.Owner.Login, *pullRequest.Base.Repo.Name, *pullRequest.Number, allLabels)
if err != nil {
return fmt.Errorf("error adding labels to pull request %d: %w", *pullRequest.Number, err) // Ensure error from label adding is handled

if !hasLabel {
allLabels := []string{label}
for _, labelP := range pullRequest.Labels {
allLabels = append(allLabels, *labelP.Name)
}
_, _, err := githubClient.Issues.AddLabelsToIssue(context.TODO(), *pullRequest.Base.Repo.Owner.Login, *pullRequest.Base.Repo.Name, *pullRequest.Number, allLabels)
if err != nil {
return fmt.Errorf("error adding labels to pull request %d: %w", *pullRequest.Number, err) // Ensure error from label adding is handled
}
}
}
}
}

return nil
}