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

fix(repo): repair update name #1011

Merged
merged 12 commits into from
Dec 18, 2023
Merged

fix(repo): repair update name #1011

merged 12 commits into from
Dec 18, 2023

Conversation

timhuynh94
Copy link
Contributor

This change is to fix the Repair button under Settings. Previously, when clicking Repair button on an unarchived renamed repo and push to renamed repo, builds will not be triggered based on webhook signatures failures.
Closes #862

@timhuynh94 timhuynh94 marked this pull request as ready for review November 22, 2023 15:09
@timhuynh94 timhuynh94 requested a review from a team as a code owner November 22, 2023 15:09
Copy link

codecov bot commented Nov 22, 2023

Codecov Report

Merging #1011 (a9fd0bb) into main (140d2b1) will not change coverage.
The diff coverage is n/a.

Additional details and impacted files

Impacted file tree graph

@@           Coverage Diff           @@
##             main    #1011   +/-   ##
=======================================
  Coverage   67.79%   67.79%           
=======================================
  Files         318      318           
  Lines       13754    13754           
=======================================
  Hits         9325     9325           
  Misses       3956     3956           
  Partials      473      473           

claire1618
claire1618 previously approved these changes Nov 22, 2023
plyr4
plyr4 previously approved these changes Nov 22, 2023
wass3rw3rk
wass3rw3rk previously approved these changes Nov 22, 2023
api/repo/repair.go Outdated Show resolved Hide resolved
@wass3rw3rk wass3rw3rk changed the title fix(server): repair update name fix(repo): repair update name Nov 22, 2023
Copy link
Contributor

@ecrupper ecrupper left a comment

Choose a reason for hiding this comment

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

Looks like a great way to further utilize the repair API. I noted a few extra steps that will be necessary for this to work as intended. Out of scope for this PR, but I wonder if the sync endpoint could also do something similar in these odd cases.

api/repo/repair.go Show resolved Hide resolved
api/repo/repair.go Outdated Show resolved Hide resolved
api/repo/repair.go Outdated Show resolved Hide resolved
api/repo/repair.go Show resolved Hide resolved
api/webhook/post.go Show resolved Hide resolved
api/repo/repair.go Outdated Show resolved Hide resolved
api/repo/repair.go Outdated Show resolved Hide resolved
Copy link
Contributor

@KellyMerrick KellyMerrick left a comment

Choose a reason for hiding this comment

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

Super minor request :disappear:

api/repo/repair.go Outdated Show resolved Hide resolved
Co-authored-by: Kelly Merrick <[email protected]>
Copy link
Contributor

@KellyMerrick KellyMerrick left a comment

Choose a reason for hiding this comment

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

lgtm

Copy link
Member

@wass3rw3rk wass3rw3rk left a comment

Choose a reason for hiding this comment

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

i would change the module import order in api/repo/repair.go to be closer to what it was before, but we don't currently enforce this and it's not consistent across repos either. otherwise, lgtm.

@timhuynh94 timhuynh94 merged commit 2dd31d6 into main Dec 18, 2023
11 of 12 checks passed
@timhuynh94 timhuynh94 deleted the bug_rename_repo branch December 18, 2023 16:14
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.

server: repairing a renamed (old) repo creates invalid webhook signatures
6 participants