Skip to content

Commit

Permalink
rename variables for code readability
Browse files Browse the repository at this point in the history
use "github_host" env var to fetch release notes for bosh releases bump

Co-authored-by: Nick Rohn <[email protected]>
  • Loading branch information
jajita and notrepo05 committed Oct 14, 2024
1 parent 5830ed3 commit 406c7e9
Show file tree
Hide file tree
Showing 5 changed files with 38 additions and 28 deletions.
1 change: 1 addition & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -658,6 +658,7 @@ stemcell_criteria:
- type: github
id: optional-unique-name-defaults-to-github-org-name
org: the-github-org
endpoint: $(variable "github_host")
github_token: $(variable "github_token")
```

Expand Down
38 changes: 22 additions & 16 deletions internal/commands/release_notes.go
Original file line number Diff line number Diff line change
Expand Up @@ -28,15 +28,15 @@ const releaseDateFormat = "2006-01-02"

type ReleaseNotes struct {
Options struct {
ReleaseDate string `long:"release-date" short:"d" description:"release date of the tile"`
TemplateName string `long:"template" short:"t" description:"path to template"`
GithubToken string `long:"github-token" short:"g" description:"auth token for fetching issues merged between releases" env:"GITHUB_TOKEN"`
GithubHost string `long:"github-host" description:"set this when you are using GitHub enterprise" env:"GITHUB_HOST"`
Kilnfile string `long:"kilnfile" short:"k" description:"path to Kilnfile"`
DocsFile string `long:"update-docs" short:"u" description:"path to docs file to update"`
Window string `long:"window" short:"w" description:"GA window for release notes" default:"ga"`
VariableFiles []string `long:"variables-file" short:"vf" description:"path to a file containing variables to interpolate"`
Variables []string `long:"variable" short:"vr" description:"key value pairs of variables to interpolate"`
ReleaseDate string `long:"release-date" short:"d" description:"release date of the tile"`
TemplateName string `long:"template" short:"t" description:"path to template"`
GithubIssuesServiceToken string `long:"github-token" short:"g" description:"auth token for fetching issues and milestones with labels" env:"GITHUB_TOKEN"`
GithubHost string `long:"github-host" description:"set this when you are using GitHub enterprise to fetch issues, milestones or notes for bosh releases" env:"GITHUB_HOST"`
Kilnfile string `long:"kilnfile" short:"k" description:"path to Kilnfile"`
DocsFile string `long:"update-docs" short:"u" description:"path to docs file to update"`
Window string `long:"window" short:"w" description:"GA window for release notes" default:"ga"`
VariableFiles []string `long:"variables-file" short:"vf" description:"path to a file containing variables to interpolate"`
Variables []string `long:"variable" short:"vr" description:"key value pairs of variables to interpolate"`
notes.IssuesQuery
notes.TrainstatQuery
}
Expand Down Expand Up @@ -82,10 +82,16 @@ func (r ReleaseNotes) Execute(args []string) error {
if err != nil {
return fmt.Errorf("failed to parse template variables: %s", err)
}
if varValue, ok := templateVariables["github_token"]; !ok && r.Options.GithubToken != "" {
templateVariables["github_token"] = r.Options.GithubToken
} else if ok && r.Options.GithubToken == "" {
r.Options.GithubToken = varValue.(string)
if varValue, ok := templateVariables["github_token"]; !ok && r.Options.GithubIssuesServiceToken != "" {
templateVariables["github_token"] = r.Options.GithubIssuesServiceToken
} else if ok && r.Options.GithubIssuesServiceToken == "" {
r.Options.GithubIssuesServiceToken = varValue.(string)
}

if varValue, ok := templateVariables["github_host"]; !ok && r.Options.GithubHost != "" {
templateVariables["github_host"] = r.Options.GithubHost
} else if ok && r.Options.GithubHost == "" {
r.Options.GithubHost = varValue.(string)
}

ctx := context.Background()
Expand All @@ -100,8 +106,8 @@ func (r ReleaseNotes) Execute(args []string) error {
}

var client *github.Client
if r.Options.GithubToken != "" {
client, err = gh.Client(ctx, r.Options.GithubHost, r.Options.GithubToken)
if r.Options.GithubIssuesServiceToken != "" {
client, err = gh.Client(ctx, r.Options.GithubHost, r.Options.GithubIssuesServiceToken)
if err != nil {
return fmt.Errorf("failed to setup github client: %w", err)
}
Expand Down Expand Up @@ -237,7 +243,7 @@ func (r ReleaseNotes) checkInputs(nonFlagArgs []string) error {
}
}

if r.Options.GithubToken == "" &&
if r.Options.GithubIssuesServiceToken == "" &&
(r.Options.IssueMilestone != "" ||
len(r.Options.IssueIDs) > 0 ||
len(r.Options.IssueLabels) > 0) {
Expand Down
2 changes: 1 addition & 1 deletion internal/commands/release_notes_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -104,7 +104,7 @@ func TestReleaseNotes_Execute(t *testing.T) {
},
}

rn.Options.GithubToken = "secret"
rn.Options.GithubIssuesServiceToken = "secret"

err := rn.Execute([]string{
"--kilnfile=tile/Kilnfile",
Expand Down
23 changes: 13 additions & 10 deletions pkg/cargo/bump.go
Original file line number Diff line number Diff line change
Expand Up @@ -129,10 +129,11 @@ type repositoryReleaseLister interface {

type (
RepositoryReleaseLister = repositoryReleaseLister
githubClientFunc func(ctx context.Context, kilnfile Kilnfile, lock BOSHReleaseTarballLock) (repositoryReleaseLister, error)
githubReleasesClient func(ctx context.Context, kilnfile Kilnfile, lock BOSHReleaseTarballLock) (repositoryReleaseLister, error)
)

func releaseNotes(ctx context.Context, kf Kilnfile, list BumpList, client githubClientFunc) (BumpList, error) {
// Fetch release notes for each of the release bumps in the Kilnfile
func releaseNotes(ctx context.Context, kf Kilnfile, list BumpList, getGithubRepositoryClientForRelease githubReleasesClient) (BumpList, error) {
const workerCount = 10

type fetchReleaseNotesForBump struct {
Expand All @@ -152,7 +153,7 @@ func releaseNotes(ctx context.Context, kf Kilnfile, list BumpList, client github
go func() {
defer wg.Done()
for j := range in {
j.bump = fetchReleasesForBump(ctx, kf, j.bump, client)
j.bump = fetchReleasesForBump(ctx, kf, j.bump, getGithubRepositoryClientForRelease)
results <- j
}
}()
Expand Down Expand Up @@ -184,10 +185,10 @@ func releaseNotes(ctx context.Context, kf Kilnfile, list BumpList, client github
}

func ReleaseNotes(ctx context.Context, kf Kilnfile, list BumpList) (BumpList, error) {
return releaseNotes(ctx, kf, list, listerForRelease(kf))
return releaseNotes(ctx, kf, list, getGithubRepositoryClientForRelease(kf))
}

func listerForRelease(kf Kilnfile) func(ctx context.Context, _ Kilnfile, lock BOSHReleaseTarballLock) (repositoryReleaseLister, error) {
func getGithubRepositoryClientForRelease(kf Kilnfile) func(ctx context.Context, _ Kilnfile, lock BOSHReleaseTarballLock) (repositoryReleaseLister, error) {
return func(ctx context.Context, kilnfile Kilnfile, lock BOSHReleaseTarballLock) (repositoryReleaseLister, error) {
spec, err := kf.BOSHReleaseTarballSpecification(lock.Name)
if err != nil {
Expand All @@ -214,7 +215,8 @@ func listerForRelease(kf Kilnfile) func(ctx context.Context, _ Kilnfile, lock BO
}
}

func fetchReleasesFromRepo(ctx context.Context, repoService RepositoryReleaseLister, repository string, from, to *semver.Version) []*github.RepositoryRelease {
// Fetch all the releases from GitHub repository between from and to versions
func fetchReleasesFromRepo(ctx context.Context, releaseLister RepositoryReleaseLister, repository string, from, to *semver.Version) []*github.RepositoryRelease {
owner, repo, err := gh.RepositoryOwnerAndNameFromPath(repository)
if err != nil {
return nil
Expand All @@ -223,7 +225,7 @@ func fetchReleasesFromRepo(ctx context.Context, repoService RepositoryReleaseLis
var result []*github.RepositoryRelease

ops := github.ListOptions{}
releases, _, err := repoService.ListReleases(ctx, owner, repo, &ops)
releases, _, err := releaseLister.ListReleases(ctx, owner, repo, &ops)
if err != nil {
log.Println(err)
}
Expand All @@ -239,7 +241,7 @@ func fetchReleasesFromRepo(ctx context.Context, repoService RepositoryReleaseLis
return result
}

func fetchReleasesForBump(ctx context.Context, kf Kilnfile, bump Bump, client githubClientFunc) Bump {
func fetchReleasesForBump(ctx context.Context, kf Kilnfile, bump Bump, getGithubRepositoryClientForRelease githubReleasesClient) Bump {
spec, err := kf.BOSHReleaseTarballSpecification(bump.Name)
if err != nil {
return bump
Expand All @@ -250,7 +252,8 @@ func fetchReleasesForBump(ctx context.Context, kf Kilnfile, bump Bump, client gi
return bump
}

lister, err := client(ctx, kf, bump.To)
// Fetch the GitHub releases client for a single release (bump) in the Kilnfile
releaseLister, err := getGithubRepositoryClientForRelease(ctx, kf, bump.To)
if err != nil {
log.Println(err)
return bump
Expand All @@ -262,7 +265,7 @@ func fetchReleasesForBump(ctx context.Context, kf Kilnfile, bump Bump, client gi
}

if spec.GitHubRepository != "" {
releases := fetchReleasesFromRepo(ctx, lister, spec.GitHubRepository, from, to)
releases := fetchReleasesFromRepo(ctx, releaseLister, spec.GitHubRepository, from, to)
bump.Releases = append(bump.Releases, releases...)
}

Expand Down
2 changes: 1 addition & 1 deletion pkg/notes/notes_data.go
Original file line number Diff line number Diff line change
Expand Up @@ -357,7 +357,7 @@ type issuesService interface {
// manual test to ensure it continues to behave as expected during refactors.
//
// The function can be tested by generating release notes for a tile with issue ids and a milestone set. The happy path
// test for Execute does not set GithubToken intentionally so this code is not triggered and Execute does not actually
// test for Execute does not set GithubIssuesServiceToken intentionally so this code is not triggered and Execute does not actually
// reach out to GitHub.
func (r fetchNotesData) fetchIssuesAndReleaseNotes(ctx context.Context, finalKF, wtKF cargo.Kilnfile, bumpList cargo.BumpList, issuesQuery IssuesQuery) ([]*github.Issue, cargo.BumpList, error) {
if r.issuesService == nil {
Expand Down

0 comments on commit 406c7e9

Please sign in to comment.