argo-cd icon indicating copy to clipboard operation
argo-cd copied to clipboard

feat(appset): add SCM-Manager to Pull Request and SCM Provider Generators

Open eheimbuch opened this issue 1 year ago • 6 comments

This PR implements the functionality of Pull Request Generator for SCM-Manager Repos. Also we added SCM-Manager to be available as SCM Provider.

Checklist:

  • [x] 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.
  • [x] The title of the PR states what changed and the related issues number (used for the release note).
  • [x] The title of the PR conforms to the Toolchain Guide
  • [x] 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.
  • [x] Does this PR require documentation updates?
  • [x] I've updated documentation as required by this PR.
  • [ ] Optional. My organization is added to USERS.md.
  • [x] I have signed off all my commits as required by DCO
  • [x] I have written unit and/or e2e tests for my change. PRs without these are unlikely to be merged.
  • [x] My build is green (troubleshooting builds).
  • [x] My new feature complies with the feature status guidelines.
  • [x] I have added a brief description of why this PR is necessary and/or what this PR solves.

Please see Contribution FAQs if you have questions about your pull-request.

eheimbuch avatar Jul 04 '23 10:07 eheimbuch

Codecov Report

Attention: Patch coverage is 63.46154% with 57 lines in your changes missing coverage. Please review.

Project coverage is 55.92%. Comparing base (44072bb) to head (9935cf9).

Files Patch % Lines
...pplicationset/services/scm_provider/scm-manager.go 79.12% 10 Missing and 9 partials :warning:
applicationset/generators/scm_provider.go 0.00% 14 Missing :warning:
applicationset/generators/pull_request.go 0.00% 12 Missing :warning:
...pplicationset/services/pull_request/scm-manager.go 77.14% 4 Missing and 4 partials :warning:
.../apis/application/v1alpha1/applicationset_types.go 0.00% 4 Missing :warning:
Additional details and impacted files
@@            Coverage Diff             @@
##           master   #14332      +/-   ##
==========================================
+ Coverage   55.85%   55.92%   +0.07%     
==========================================
  Files         316      318       +2     
  Lines       43763    43919     +156     
==========================================
+ Hits        24445    24563     +118     
- Misses      16765    16795      +30     
- Partials     2553     2561       +8     

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

codecov[bot] avatar Jul 05 '23 09:07 codecov[bot]

Hi @ishitasequeira !

Should we also provide the ability for the user to configure a cert bundle on the appset controller so that users don't have to disable cert validation similar to Gitlab SCMProvider?

We would skip this feature from our side for now and would be very greatful to see this first step going live. Thanks for your review, we highly appreciate this.

pfeuffer avatar Sep 14 '23 18:09 pfeuffer

Hi @ishitasequeira,

we fixed your review some time ago and still waiting for your feedback / pr merge. We have more integrations for SCM Manager with ArgoCD in our pipeline, but are hesitant due to the status of this pull request.

eheimbuch avatar Oct 23 '23 07:10 eheimbuch

Bump @ishitasequeira

eheimbuch avatar Nov 07 '23 07:11 eheimbuch

Hi @ishitasequeira , thanks for the feedback. We will add this and report back when we're finished.

pfeuffer avatar Dec 18 '23 08:12 pfeuffer

Hey @ishitasequeira , thanks for the patience! We've added the cert validation and it would be great if you could take a look again.

pfeuffer avatar Aug 21 '24 09:08 pfeuffer