Skip to content

Rename ListCommitsWithNotes and make method private #1070

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.

Sign up for GitHub

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 1 commit into from
Feb 6, 2020
Merged

Rename ListCommitsWithNotes and make method private #1070

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
21 changes: 9 additions & 12 deletions pkg/notes/notes.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -164,7 +164,7 @@ func (g *Gatherer) ListReleaseNotes() (ReleaseNotes, ReleaseNotesHistory, error)
return nil, nil, err
}

results, err := g.ListCommitsWithNotes(commits)
results, err := g.gatherNotes(commits)
if err != nil {
return nil, nil, err
}
Expand Down Expand Up @@ -503,13 +503,10 @@ func matchesIncludeFilter(msg string) *regexp.Regexp {
return matchesFilter(msg, noteInclusionFilters)
}

// ListCommitsWithNotes list commits that have release notes starting from a
// given commit SHA and ending at a given commit SHA. This function is similar
// to ListCommits except that only commits with tagged release notes are
// returned.
//TODO: This name does not make sense anymore
//TODO: Why is that method exported?
func (g *Gatherer) ListCommitsWithNotes(commits []*github.RepositoryCommit) (filtered []*Result, err error) {
// gatherNotes list commits that have release notes starting from a given
// commit SHA and ending at a given commit SHA. This function is similar to
// ListCommits except that only commits with tagged release notes are returned.
func (g *Gatherer) gatherNotes(commits []*github.RepositoryCommit) (filtered []*Result, err error) {
allResults := &resultList{}

nrOfCommits := len(commits)
Expand Down Expand Up @@ -565,7 +562,7 @@ func (g *Gatherer) notesForCommit(commit *github.RepositoryCommit) (*Result, err
if err != nil {
if err == errNoPRIDFoundInCommitMessage || err == errNoPRFoundForCommitSHA {
logrus.
WithField("func", "ListCommitsWithNotes").
WithField("func", "listCommitsWithNotes").
Debugf("No matches found when parsing PR from commit sha %q", commit.GetSHA())
return nil, nil
}
Expand All @@ -576,14 +573,14 @@ func (g *Gatherer) notesForCommit(commit *github.RepositoryCommit) (*Result, err
prBody := pr.GetBody()

logrus.
WithField("func", "ListCommitsWithNotes").
WithField("func", "listCommitsWithNotes").
WithField("pr no", pr.GetNumber()).
WithField("pr body", pr.GetBody()).
Debugf("Obtaining PR associated with commit sha %q", commit.GetSHA())

if re := matchesExcludeFilter(prBody); re != nil {
logrus.
WithField("func", "ListCommitsWithNotes").
WithField("func", "listCommitsWithNotes").
WithField("filter", re.String()).
Debug("Excluding notes for PR based on the exclusion filter.")
// try next PR
Expand All @@ -593,7 +590,7 @@ func (g *Gatherer) notesForCommit(commit *github.RepositoryCommit) (*Result, err
if re := matchesIncludeFilter(prBody); re != nil {
res := &Result{commit: commit, pullRequest: pr}
logrus.
WithField("func", "ListCommitsWithNotes").
WithField("func", "listCommitsWithNotes").
WithField("filter", re.String()).
Debug("Including notes for PR based on the inclusion filter.")

Expand Down
19 changes: 9 additions & 10 deletions pkg/notes/notes_gatherer_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ See the License for the specific language governing permissions and
limitations under the License.
*/

package notes_test
package notes

import (
"context"
Expand All @@ -31,7 +31,6 @@ import (
"github.com/google/go-github/v28/github"
"github.com/sirupsen/logrus"
"k8s.io/release/pkg/git"
"k8s.io/release/pkg/notes"
"k8s.io/release/pkg/notes/client/clientfakes"
)

Expand Down Expand Up @@ -204,7 +203,7 @@ func TestListCommits(t *testing.T) {
}
}

gatherer := notes.NewGathererWithClient(context.Background(), client)
gatherer := NewGathererWithClient(context.Background(), client)
commits, err := gatherer.ListCommits(tc.branch, tc.start, tc.end)

checkErrMsg(t, err, tc.expectedErrMsg)
Expand Down Expand Up @@ -235,7 +234,7 @@ func TestListCommits(t *testing.T) {
}
}

func TestListCommitsWithNotes(t *testing.T) {
func TestGatherNotes(t *testing.T) {
type getPullRequestStub func(context.Context, string, string, int) (*github.PullRequest, *github.Response, error)
type listPullRequestsWithCommitStub func(context.Context, string, string, string, *github.PullRequestListOptions) ([]*github.PullRequest, *github.Response, error)

Expand All @@ -253,7 +252,7 @@ func TestListCommitsWithNotes(t *testing.T) {
// called with and to inject faked return data.
getPullRequestStubber func(*testing.T) getPullRequestStub

// commitList is the list of commits the ListCommitsWithNotes is acting on
// commitList is the list of commits the gatherNotes is acting on
commitList []*github.RepositoryCommit

// expectedErrMsg is the error message the method is expected to return
Expand All @@ -266,9 +265,9 @@ func TestListCommitsWithNotes(t *testing.T) {
expectedGetPullRequestCallCount int

// resultChecker is a function which gets called with the result of
// ListCommitsWithNotes, giving users the option to validate the returned
// gatherNotes, giving users the option to validate the returned
// data
resultsChecker func(*testing.T, []*notes.Result)
resultsChecker func(*testing.T, []*Result)
}{
"empty commit list": {
// Does not call anything
Expand Down Expand Up @@ -368,7 +367,7 @@ func TestListCommitsWithNotes(t *testing.T) {
}
},
expectedListPullRequestsWithCommitCallCount: 20,
resultsChecker: func(t *testing.T, results []*notes.Result) {
resultsChecker: func(t *testing.T, results []*Result) {
// there is not much we can check on the Result, as all the fields are
// unexported
expectedResultSize := 7
Expand All @@ -386,15 +385,15 @@ func TestListCommitsWithNotes(t *testing.T) {

client := &clientfakes.FakeClient{}

gatherer := notes.NewGathererWithClient(context.Background(), client)
gatherer := NewGathererWithClient(context.Background(), client)
if stubber := tc.listPullRequestsWithCommitStubber; stubber != nil {
client.ListPullRequestsWithCommitStub = stubber(t)
}
if stubber := tc.getPullRequestStubber; stubber != nil {
client.GetPullRequestStub = stubber(t)
}

results, err := gatherer.ListCommitsWithNotes(tc.commitList)
results, err := gatherer.gatherNotes(tc.commitList)

checkErrMsg(t, err, tc.expectedErrMsg)

Expand Down