Skip to content

fix: --skip-archived-repos does not filter correctly #157

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

Closed
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
10 changes: 10 additions & 0 deletions mocks/mocks.go
Original file line number Diff line number Diff line change
@@ -16,13 +16,15 @@ var (
repoName2 = "terratest"
repoName3 = "fetch"
repoName4 = "terraform-kubernetes-helm"
repoName5 = "terraform-google-load-balancer"
)

var (
repoURL1 = "https://github.com/gruntwork-io/terragrunt"
repoURL2 = "https://github.com/gruntwork-io/terratest"
repoURL3 = "https://github.com/gruntwork-io/fetch"
repoURL4 = "https://github.com/gruntwork-io/terraform-kubernetes-helm"
repoURL5 = "https://github.com/gruntwork-io/terraform-google-load-balancer"
)

var archivedFlag = true
@@ -57,6 +59,14 @@ var MockGithubRepositories = []*github.Repository{
HTMLURL: &repoURL4,
Archived: &archivedFlag,
},
{
Owner: &github.User{
Login: &ownerName,
},
Name: &repoName5,
HTMLURL: &repoURL5,
Archived: &archivedFlag,
},
}

// This mocks the PullRequest service in go-github that is used in production to call the associated GitHub endpoint
8 changes: 3 additions & 5 deletions repository/fetch-repos.go
Original file line number Diff line number Diff line change
@@ -91,19 +91,17 @@ func getReposByOrg(config *config.GitXargsConfig) ([]*github.Repository, error)
}

// github.RepositoryListByOrgOptions doesn't seem to be able to filter out archived repos
// So re-slice the repos list if --skip-archived-repos is passed and the repository is in archived/read-only state
for i, repo := range repos {
// So filter the repos list if --skip-archived-repos is passed and the repository is in archived/read-only state
for _, repo := range repos {
Copy link
Contributor

Choose a reason for hiding this comment

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

if config.SkipArchivedRepos {
	for _, repo := range repos {
		if repo.GetArchived() {
			logger.WithFields(logrus.Fields{
				"Name": repo.GetFullName(),
			}).Debug("Skipping archived repository")

			// Track repos to skip because of archived status for our final run report
			config.Stats.TrackSingle(stats.ReposArchivedSkipped, repo)
		} else {
			reposToAdd = append(reposToAdd, repo)
		}
	}
} else {
	reposToAdd = repos
}	

how about checking for the flag first, cut down on the extra reassignments if config.SkipArchivedRepos == false?

if config.SkipArchivedRepos && repo.GetArchived() {
logger.WithFields(logrus.Fields{
"Name": repo.GetFullName(),
}).Debug("Skipping archived repository")

// Track repos to skip because of archived status for our final run report
config.Stats.TrackSingle(stats.ReposArchivedSkipped, repo)

reposToAdd = append(repos[:i], repos[i+1:]...)
} else {
reposToAdd = repos
reposToAdd = append(reposToAdd, repo)
}
}

2 changes: 1 addition & 1 deletion repository/fetch-repos_test.go
Original file line number Diff line number Diff line change
@@ -63,6 +63,6 @@ func TestSkipArchivedRepos(t *testing.T) {

githubRepos, reposByOrgLookupErr := getReposByOrg(config)

assert.Equal(t, len(githubRepos), len(mocks.MockGithubRepositories)-1)
assert.Equal(t, len(githubRepos), len(mocks.MockGithubRepositories)-2)
assert.NoError(t, reposByOrgLookupErr)
}