Skip to content

fix: copy github app key from repocreds#6197

Merged
jannfis merged 2 commits into
argoproj:masterfrom
joebowbeer:patch-1
May 11, 2021
Merged

fix: copy github app key from repocreds#6197
jannfis merged 2 commits into
argoproj:masterfrom
joebowbeer:patch-1

Conversation

@joebowbeer

@joebowbeer joebowbeer commented May 10, 2021 •
edited
Loading

Copy link
Copy Markdown
Contributor

Fixes #6196
Closes #3086

This PR completes the fix for a prior regression that went undetected.

Checklist:

  • This is a bug fix.
  • The title of the PR states what changed.
  • I've included "Closes [ISSUE #]" or "Fixes [ISSUE #]" in the description to automatically close the associated issue.
  • 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).

@joebowbeer
fix: copy github app key from repocreds
3c0cba0 
Fixes argoproj#6196

Signed-off-by: Joe Bowbeer <joe.bowbeer@gmail.com>
@codecov

codecov Bot commented May 10, 2021 •
edited
Loading

Copy link
Copy Markdown

Codecov Report

Merging #6197 (daca7e6) into master (5ec7d6e) will decrease coverage by 0.01%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #6197      +/-   ##
==========================================
- Coverage   41.21%   41.20%   -0.02%     
==========================================
  Files         147      147              
  Lines       19850    19858       +8     
==========================================
+ Hits         8181     8182       +1     
- Misses      10549    10556       +7     
  Partials     1120     1120
Impacted Files Coverage Δ
util/db/repository.go 62.73% <100.00%> (+0.11%) ⬆️
cmd/argocd/commands/cluster.go 2.59% <0.00%> (-0.10%) ⬇️
util/settings/settings.go 46.75% <0.00%> (ø)

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 5ec7d6e...daca7e6. Read the comment docs.

@joebowbeer
Add GitHub App to unit test
daca7e6 
Signed-off-by: Joe Bowbeer <joe.bowbeer@gmail.com>
jannfis
jannfis approved these changes May 11, 2021
View reviewed changes

@jannfis jannfis left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM, thank you @joebowbeer !

@jannfis jannfis merged commit 31d2345 into argoproj:master May 11, 2021
@joebowbeer joebowbeer deleted the patch-1 branch May 11, 2021 07:58
This was referenced May 12, 2021
Closed
Closed
@jannfis jannfis added the cherry-pick/2.0 Candidate for cherry picking into the 2.0 release branch label May 13, 2021
jannfis pushed a commit that referenced this pull request May 15, 2021
@joebowbeer @jannfis
fix: copy github app key from repocreds (#6197)
beb9370 
* fix: copy github app key from repocreds

Fixes #6196

Signed-off-by: Joe Bowbeer <joe.bowbeer@gmail.com>

* Add GitHub App to unit test

Signed-off-by: Joe Bowbeer <joe.bowbeer@gmail.com>
@jannfis

jannfis commented May 15, 2021

Copy link
Copy Markdown
Member

This commit has been cherry-picked into 2.0 release branch.

/cc @alexmt

@jannfis jannfis self-assigned this Jun 25, 2021
@jannfis jannfis added the needs-verification PR requires pre-release verification label Jun 25, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@jannfis jannfis jannfis approved these changes

Assignees

@jannfis jannfis

Labels

cherry-pick/2.0 Candidate for cherry picking into the 2.0 release branch needs-verification PR requires pre-release verification

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

GitHub App auth regression Support private GitHub repo authentication using GitHub app

2 participants

@joebowbeer @jannfis