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

Add helper #7

Closed
wants to merge 3 commits into from
Closed
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
3 changes: 3 additions & 0 deletions cmd/notifyPendingCI.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
package cmd

// For future use
47 changes: 47 additions & 0 deletions cmd/notifyStale.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,47 @@
package cmd

//import (
// "log"
// "time"
//
// "github.com/google/go-github/v60/github"
// "github.com/spf13/cobra"
// "github.com/spf13/viper"
//
// "github.com/chia-network/github-bot/internal/config"
// "github.com/chia-network/github-bot/internal/label"
//)
//
//labelPRsCmd represents the labelPRs command
//var sendKeybaseMessage = &cobra.Command{
// Use: "sendKeybaseMessage",
// Short: "Sends a message to Keybase informing users of stale community PRs",
// Run: func(cmd *cobra.Command, args []string) {
// cfg, err := config.LoadConfig(viper.GetString("config"))
// if err != nil {
// log.Fatalf("error loading config: %s\n", err.Error())
// }
// client := github.NewClient(nil).WithAuthToken(cfg.GithubToken)
//
// loop := viper.GetBool("loop")
// loopDuration := viper.GetDuration("loop-time")
// for {
// log.Println("Labeling Pull Requests")
// err = label.PullRequests(client, cfg.InternalTeam, cfg.LabelConfig)
// if err != nil {
// log.Fatalln(err.Error())
// }
//
// if !loop {
// break
// }
//
// log.Printf("Waiting %s for next iteration\n", loopDuration.String())
// time.Sleep(loopDuration)
// }
// },
//}
//
//func init() {
// rootCmd.AddCommand(sendKeybaseMessage)
//}
5 changes: 4 additions & 1 deletion cmd/root.go
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,9 @@ func initConfig() {

// If a config file is found, read it in.
if err := viper.ReadInConfig(); err == nil {
fmt.Fprintln(os.Stderr, "Using config file:", viper.ConfigFileUsed())
_, err := fmt.Fprintln(os.Stderr, "Using config file:", viper.ConfigFileUsed())
if err != nil {
return
}
}
}
41 changes: 41 additions & 0 deletions internal/github/pullRequests.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
package github

import (
"context"
"fmt"

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

// FindCommunityPRs obtains non-teammember PRs
func FindCommunityPRs(owner string, repo string, teamMembers map[string]bool, githubClient *github.Client) ([]*github.PullRequest, error) {
var finalPRs []*github.PullRequest
opts := &github.PullRequestListOptions{
State: "open",
Sort: "created",
Direction: "desc",
ListOptions: github.ListOptions{
Page: 0,
PerPage: 100,
},
}
for {
opts.ListOptions.Page++
pullRequests, resp, err := githubClient.PullRequests.List(context.TODO(), owner, repo, opts)
if err != nil {
return finalPRs, fmt.Errorf("error listing pull requests: %w", err)
}

for _, pullRequest := range pullRequests {
user := *pullRequest.User.Login
if !teamMembers[user] {
finalPRs = append(finalPRs, pullRequest)
}
}

if resp.NextPage == 0 {
break
}
}
return finalPRs, nil
}
44 changes: 44 additions & 0 deletions internal/github/teamMemList.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,44 @@
package github

import (
"context"
"fmt"
"strings"

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

// GetTeamMemList obtains a list of teammembers
func GetTeamMemList(githubClient *github.Client, internalTeam string) (map[string]bool, error) {
Comment on lines +11 to +12
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
// GetTeamMemList obtains a list of teammembers
func GetTeamMemList(githubClient *github.Client, internalTeam string) (map[string]bool, error) {
// GetTeamMemberList obtains a list of teammembers
func GetTeamMemberList(githubClient *github.Client, internalTeam string) (map[string]bool, error) {

More clear to just use full names. No penalties for a slightly longer function name here

teamMembers := make(map[string]bool)

teamParts := strings.Split(internalTeam, "/")
if len(teamParts) != 2 {
return nil, fmt.Errorf("invalid team name - must contain org and team : %s", internalTeam)
}

teamOpts := &github.TeamListTeamMembersOptions{
Role: "all",
ListOptions: github.ListOptions{
Page: 0,
PerPage: 100,
},
}

for {
teamOpts.ListOptions.Page++
members, resp, err := githubClient.Teams.ListTeamMembersBySlug(context.TODO(), teamParts[0], teamParts[1], teamOpts)
if err != nil {
return nil, fmt.Errorf("error getting team %s: %w", internalTeam, err)
}

for _, member := range members {
teamMembers[*member.Login] = true
}

if resp.NextPage == 0 {
break
}
}
return teamMembers, nil
}
134 changes: 47 additions & 87 deletions internal/label/pullrequests.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,114 +9,74 @@ import (
"github.com/google/go-github/v60/github"

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

// PullRequests applies internal or community labels to pull requests
// Internal is determined by checking if the PR author is a member of the specified internalTeam
func PullRequests(githubClient *github.Client, internalTeam string, cfg config.LabelConfig) error {
teamMembers := map[string]bool{}

teamParts := strings.Split(internalTeam, "/")
if len(teamParts) != 2 {
return fmt.Errorf("invalid team name - must contain org and team : %s", internalTeam)
}

teamOpts := &github.TeamListTeamMembersOptions{
Role: "all",
ListOptions: github.ListOptions{
Page: 0,
PerPage: 100,
},
}

for {
teamOpts.ListOptions.Page++
members, resp, err := githubClient.Teams.ListTeamMembersBySlug(context.TODO(), teamParts[0], teamParts[1], teamOpts)
if err != nil {
return fmt.Errorf("error getting team %s: %w", internalTeam, err)
}

for _, member := range members {
teamMembers[*member.Login] = true
}

if resp.NextPage == 0 {
break
}
}

for _, fullRepo := range cfg.LabelCheckRepos {
log.Println("checking repos")
parts := strings.Split(fullRepo.Name, "/")
if len(parts) != 2 {
return fmt.Errorf("invalid repository name - must contain owner and repository: %s", fullRepo.Name)
}
opts := &github.PullRequestListOptions{
State: "open",
Sort: "created",
Direction: "desc",
ListOptions: github.ListOptions{
Page: 0,
PerPage: 100,
},
owner := parts[0]
repo := parts[1]
teamMembers, _ := github2.GetTeamMemList(githubClient, internalTeam)
Copy link
Contributor

Choose a reason for hiding this comment

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

This should not be inside of any loops - put it first thing in this function out of the loop. The team member list doesn't change based on what repo we're lookign at, and by doing it in the loop, we're rebuilding this list over and over for no gain (and a cost of a bunch of extra API calls and time)

pullRequests, err := github2.FindCommunityPRs(owner, repo, teamMembers, githubClient)
if err != nil {
return err
}
for {
lowestNumber := 0
opts.ListOptions.Page++
owner := parts[0]
repo := parts[1]
pullRequests, resp, err := githubClient.PullRequests.List(context.TODO(), owner, repo, opts)
if err != nil {
return fmt.Errorf("error listing pull requests: %w", err)

lowestNumber := 0

for _, pullRequest := range pullRequests {
lowestNumber = *pullRequest.Number
if *pullRequest.Number < fullRepo.MinimumNumber {
// Break, not continue, since our order ensures PR numbers are getting smaller
break
}
if *pullRequest.Draft {
continue
}
user := *pullRequest.User.Login
if cfg.LabelSkipMap[user] {
continue
}
var label string
if teamMembers[user] {
label = cfg.LabelInternal
} else {
label = cfg.LabelExternal
}

for _, pullRequest := range pullRequests {
lowestNumber = *pullRequest.Number
if *pullRequest.Number < fullRepo.MinimumNumber {
// Break, not continue, since our order ensures PR numbers are getting smaller
break
}
if *pullRequest.Draft {
continue
}
user := *pullRequest.User.Login
if cfg.LabelSkipMap[user] {
continue
}
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
}
}

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

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

if resp.NextPage == 0 || lowestNumber <= fullRepo.MinimumNumber {
break
}
if lowestNumber <= fullRepo.MinimumNumber {
break
Copy link
Contributor

Choose a reason for hiding this comment

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

In this case, I think we just need to continue instead of break so that we can look at the other PRs

}
}

Expand Down
3 changes: 3 additions & 0 deletions internal/message/sendMessage.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
package message

// For Future Use