Skip to content

Commit

Permalink
Allow the same reviewer in both core and cloud repos
Browse files Browse the repository at this point in the history
  • Loading branch information
jimbishopp committed Dec 28, 2023
1 parent fabb820 commit d85387b
Show file tree
Hide file tree
Showing 4 changed files with 39 additions and 4 deletions.
8 changes: 8 additions & 0 deletions bot/internal/env/env.go
Original file line number Diff line number Diff line change
Expand Up @@ -132,6 +132,14 @@ func (e *Environment) IsCloudDeployBranch() bool {
(e.UnsafeBase == CloudProdBranch || e.UnsafeBase == CloudStagingBranch)
}

// Team returns CloudTeam when the repository is the cloud repo otherwise CoreTeam.
func (e *Environment) Team() string {
if e.Repository == CloudRepo {
return CloudTeam
}
return CoreTeam
}

func readEvent() (*Event, error) {
f, err := os.Open(os.Getenv(githubEventPath))
if err != nil {
Expand Down
18 changes: 17 additions & 1 deletion bot/internal/review/review.go
Original file line number Diff line number Diff line change
Expand Up @@ -109,6 +109,8 @@ type Config struct {
// CodeReviewers and CodeReviewersOmit is a map of code reviews and code
// reviewers to omit.
CodeReviewers map[string]Reviewer `json:"codeReviewers"`
CoreReviewers map[string]Reviewer `json:"coreReviewers"`
CloudReviewers map[string]Reviewer `json:"cloudReviewers"`
CodeReviewersOmit map[string]bool `json:"codeReviewersOmit"`

// DocsReviewers and DocsReviewersOmit is a map of docs reviews and docs
Expand Down Expand Up @@ -156,12 +158,26 @@ type Assignments struct {
}

// FromString parses JSON formatted configuration and returns assignments.
func FromString(reviewers string) (*Assignments, error) {
func FromString(e *env.Environment, reviewers string) (*Assignments, error) {
var c Config
if err := json.Unmarshal([]byte(reviewers), &c); err != nil {
return nil, trace.Wrap(err)
}

// hacky way of allowing the same reviewer to be defined in
// both cloud and core repos without having to change the
// bot in a large number of places.
if len(c.CodeReviewers) == 0 { // allow this change to deploy before updating the config file
switch e.Team() {
case env.CloudTeam:
c.CodeReviewers = c.CloudReviewers
case env.CoreTeam:
c.CodeReviewers = c.CoreReviewers
default:
return nil, trace.BadParameter("unable to detect code reviewers due to invalid team: %s", e.Team())
}
}

r, err := New(&c)
if err != nil {
return nil, trace.Wrap(err)
Expand Down
15 changes: 13 additions & 2 deletions bot/internal/review/review_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1074,7 +1074,8 @@ func TestCheckInternal(t *testing.T) {

// TestFromString tests if configuration is correctly read in from a string.
func TestFromString(t *testing.T) {
r, err := FromString(reviewers)
e := &env.Environment{Repository: env.TeleportRepo}
r, err := FromString(e, reviewers)
require.NoError(t, err)

require.EqualValues(t, r.c.CodeReviewers, map[string]Reviewer{
Expand Down Expand Up @@ -1194,7 +1195,7 @@ func TestPreferredReviewers(t *testing.T) {

const reviewers = `
{
"codeReviewers": {
"coreReviewers": {
"1": {
"team": "Core",
"owner": true
Expand All @@ -1204,6 +1205,16 @@ const reviewers = `
"owner": false
}
},
"cloudReviewers": {
"1": {
"team": "Cloud",
"owner": true
},
"2": {
"team": "Cloud",
"owner": false
}
},
"codeReviewersOmit": {
"3": true
},
Expand Down
2 changes: 1 addition & 1 deletion bot/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -178,7 +178,7 @@ func createBot(ctx context.Context, flags flags) (*bot.Bot, error) {
if err != nil {
return nil, trace.Wrap(err)
}
reviewer, err := review.FromString(flags.reviewers)
reviewer, err := review.FromString(environment, flags.reviewers)
if err != nil {
return nil, trace.Wrap(err)
}
Expand Down

0 comments on commit d85387b

Please sign in to comment.