-
Notifications
You must be signed in to change notification settings - Fork 1.2k
Clean up import list #644
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
Comments
Yes this needs doing, the only thing that doesn't need migrating to the new location in the code is the summerwind API itself. |
@sledigabel Hey! Thanks a lot for reporting. I completely forgot to do this. As I was about to start working on another feature, I've submitted #645 for this to avoid potential conflict. Would you mind reviewing it? |
Well done for the quick feedback! I put a few comments around the helm charts, but the rest looks good to me |
@mumoshu @sledigabel you haven't renamed Docker Hub account yet so .Values.image.repository value is currently broken |
@jstewart612 Thanks! I'll fix it in #663 |
* Fix image repo name in chart Ref #644 (comment)
* Fix image repo name in chart Ref actions/actions-runner-controller#644 (comment)
Hi,
Not a bug per say, and sorry if this was mentioned before, I've noticed that some imports are pointing to
summerwind
rather thanactions-runner-controller
, as I understand it, it's some left overs from the rename.A few examples:
https://github.com/actions-runner-controller/actions-runner-controller/blob/master/cmd/githubwebhookserver/main.go#L28-L29
https://github.com/actions-runner-controller/actions-runner-controller/blob/master/main.go#L27-L29
and basically everywhere. I presume it still works because of the github redirection after the project was moved,
Just thought I'd file an issue to mention this, and if that makes sense I could file a quick PR to search/replace
summerwind
intoactions-runner-controller
as a first contribution to this great project?The text was updated successfully, but these errors were encountered: