Skip to content
This repository was archived by the owner on Aug 5, 2025. It is now read-only.

Conversation

@ecrupper
Copy link
Contributor

@ecrupper ecrupper commented May 9, 2022

Context: there have been some errors related to repository events not being processed correctly by the webhook endpoint. This led me to look over the renaming a repository code. The fix for the errors can be handled in the server code, but what caught my eye was the fact that I chose to override the hook.Event field to be a custom renameRepository hard-coded string. With the new scoped ruleset support, I think it would be consistent to mirror build.Event and build.EventAction with hook.Event and hook.EventAction.

Also random, but VSCode tried to include a project folder in the commit, so I added that to the .gitignore.

@ecrupper ecrupper self-assigned this May 9, 2022
@ecrupper ecrupper requested a review from a team as a code owner May 9, 2022 19:41
@codecov
Copy link

codecov bot commented May 9, 2022

Codecov Report

Merging #252 (e430625) into master (c60972a) will increase coverage by 0.00%.
The diff coverage is 100.00%.

Impacted file tree graph

@@           Coverage Diff           @@
##           master     #252   +/-   ##
=======================================
  Coverage   97.08%   97.08%           
=======================================
  Files          57       57           
  Lines        6268     6288   +20     
=======================================
+ Hits         6085     6105   +20     
  Misses        134      134           
  Partials       49       49           
Impacted Files Coverage Δ
database/hook.go 100.00% <100.00%> (ø)
library/hook.go 100.00% <100.00%> (ø)

Copy link
Contributor

@jbrockopp jbrockopp left a comment

Choose a reason for hiding this comment

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

LGTM

@ecrupper ecrupper merged commit 0cf0e47 into master May 18, 2022
@ecrupper ecrupper deleted the fix/clean-up-repository-rename-code branch May 18, 2022 15:19
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants