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

added gitlab support #1

Open
wants to merge 16 commits into
base: master
Choose a base branch
from
Open

added gitlab support #1

wants to merge 16 commits into from

Conversation

empath-nirvana
Copy link
Owner

No description provided.

Copy link

@coderanger coderanger left a comment

Choose a reason for hiding this comment

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

Overall looks good.

You also need to add a block of code here https://github.com/argoproj-labs/applicationset/blob/master/pkg/generators/scm_provider.go#L62-L71 to stitch things together between the config struct and generator struct(s).

type SCMProviderGeneratorGitlab struct {
// GitHub org to scan. Required.
Group string `json:"group"`
// The GitHub API URL to talk to. If blank, use https://api.github.com/.

Choose a reason for hiding this comment

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

Tweak the comment.

pkg/services/scm_provider/gitlab.go Outdated Show resolved Hide resolved
"github.com/stretchr/testify/assert"
)

// func checkRateLimit(t *testing.T, err error) {

Choose a reason for hiding this comment

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

Guessing you commented this because it would collide with the impl from github? Could rename that checkGithubRateLimit and this checkGitlabRateLimit since they might not use the same error messages? But "rate limit exceeded" is generic enough it might work for both by accident :D

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants