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

feat: add GitLab to PR generator for applicationset #9264

Merged
merged 1 commit into from May 19, 2022

Conversation

iverberk
Copy link
Contributor

@iverberk iverberk commented Apr 30, 2022

Closes #9262

Note on DCO:

If the DCO action in the integration test fails, one or more of your commits are not signed off. Please click on the Details link next to the DCO action for instructions on how to resolve this.

Checklist:

  • Either (a) I've created an enhancement proposal and discussed it with the community, (b) this is a bug fix, or (c) this does not need to be in the release notes: ApplicationSet Pull Request Generator for GitLab #9262 (Created by Spork)
  • The title of the PR states what changed and the related issues number (used for the release note).
  • I've included "Closes [ISSUE #]" or "Fixes [ISSUE #]" in the description to automatically close the associated issue.
  • I've updated both the CLI and UI to expose my feature, or I plan to submit a second PR with them.
  • Does this PR require documentation updates?
  • I've updated documentation as required by this PR.
  • Optional. My organization is added to USERS.md.
  • I have signed off all my commits as required by DCO
  • I have written unit and/or e2e tests for my change. PRs without these are unlikely to be merged.
  • My build is green (troubleshooting builds).

@codecov
Copy link

codecov bot commented Apr 30, 2022

Codecov Report

Merging #9264 (347bf29) into master (3337616) will increase coverage by 0.02%.
The diff coverage is 72.50%.

@@            Coverage Diff             @@
##           master    #9264      +/-   ##
==========================================
+ Coverage   45.73%   45.75%   +0.02%     
==========================================
  Files         220      221       +1     
  Lines       26006    26046      +40     
==========================================
+ Hits        11893    11917      +24     
- Misses      12456    12469      +13     
- Partials     1657     1660       +3     
Impacted Files Coverage Δ
applicationset/generators/pull_request.go 39.13% <0.00%> (-3.73%) ⬇️
...is/applicationset/v1alpha1/applicationset_types.go 34.69% <ø> (ø)
applicationset/services/pull_request/gitlab.go 85.29% <85.29%> (ø)
applicationset/services/scm_provider/github.go 75.29% <0.00%> (-5.89%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 3337616...347bf29. Read the comment docs.

@iverberk iverberk force-pushed the gitlab-pr-generator branch 6 times, most recently from a0bdba6 to c282d0b Compare May 2, 2022 11:14
@iverberk iverberk marked this pull request as ready for review May 2, 2022 11:14
Signed-off-by: Ivo Verberk <ivo.verberk@gmail.com>
}, nil
}

func (g *GitLabService) List(ctx context.Context) ([]*PullRequest, error) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Thanks

LGTM!! , just a nitpick if u feel to change then do it

Document the functions and change this function name

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I've kept the function name consistent with the existing pull request services (Github, gitea, bitbucket, etc.). What would you suggest in terms of an alternative name?

@RuBiCK
Copy link

RuBiCK commented May 12, 2022

Is there any ETA for this feature?

Copy link
Collaborator

@crenshaw-dev crenshaw-dev left a comment

Choose a reason for hiding this comment

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

Just one small nit. :-)

@crenshaw-dev
Copy link
Collaborator

@RuBiCK by default, this would end up in 2.5. If you need it in 2.4, I can check with folks about cherry-picking it back.

@RuBiCK
Copy link

RuBiCK commented May 12, 2022

@crenshaw-dev that is what we need to create ephemeral environments.
If it could be implemented in 2.4 it will be really amazing.

@crenshaw-dev
Copy link
Collaborator

@RuBiCK I'll bring it up in the contributor meeting today. https://docs.google.com/document/d/1xkoFkVviB70YBzSEa4bDnu-rUZ1sIFtwKKG1Uw8XsY8/edit

@crenshaw-dev
Copy link
Collaborator

@RuBiCK the consensus from the contributors is that we should protect the RC and save this feature for 2.5.

If you need this feature super soon, it should be easy enough to cherry-pick onto a custom build.

@RuBiCK
Copy link

RuBiCK commented May 12, 2022

@crenshaw-dev thanks for bringing that up in the meeting, I really appreciate it.

In the beginning, I thought it was an undocumented feature because it's mentioned in the documentation and I thought It would be already included and documented for 2.4.

In my company, we do not program in GO and I'm not a developer, but I'll try to find someone to help me build this version because It's crucial for the company this feature to adopt Argocd.

@crenshaw-dev
Copy link
Collaborator

In the beginning, I thought it was an undocumented feature because it's mentioned in the documentation and I thought It would be already included and documented for 2.4.

Ah yep, we need to remove that. I'm not sure how it got there.

The basic steps of building this internally are:

  1. clone the argo-cd repo and checkout the branch for the release you want to run (release-2.4 probably)
  2. git cherry-pick COMMIT onto that branch, where COMMIT is the commit has for this PR (that commit hash will appear on this PR after I merge it)
  3. make image IMAGE_TAG=2.4-plus-gitlab
  4. docker push that tag to your internal artifact repo
  5. update your Argo CD install manifests to use that image for the appset controller (no need to update the other images)
  6. kubectl apply the applicationset CRD manifest from this PR to the cluster where you need gitlab support

Copy link
Collaborator

@crenshaw-dev crenshaw-dev left a comment

Choose a reason for hiding this comment

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

Thanks!

@crenshaw-dev crenshaw-dev merged commit dd4dbe2 into argoproj:master May 19, 2022
Aym3nTN pushed a commit to Aym3nTN/argo-cd that referenced this pull request Sep 15, 2022
Signed-off-by: Ivo Verberk <ivo.verberk@gmail.com>
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.

ApplicationSet Pull Request Generator for GitLab
4 participants