Skip to content

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

Closed
sledigabel opened this issue Jun 21, 2021 · 5 comments · Fixed by #645
Closed

Clean up import list #644

sledigabel opened this issue Jun 21, 2021 · 5 comments · Fixed by #645
Labels

Comments

@sledigabel
Copy link
Contributor

Hi,

Not a bug per say, and sorry if this was mentioned before, I've noticed that some imports are pointing to summerwind rather than actions-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 into actions-runner-controller as a first contribution to this great project?

@toast-gear
Copy link
Collaborator

Yes this needs doing, the only thing that doesn't need migrating to the new location in the code is the summerwind API itself.

mumoshu added a commit that referenced this issue Jun 22, 2021
@mumoshu
Copy link
Collaborator

mumoshu commented Jun 22, 2021

@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?

mumoshu added a commit that referenced this issue Jun 22, 2021
@sledigabel
Copy link
Contributor Author

Well done for the quick feedback! I put a few comments around the helm charts, but the rest looks good to me

@jstewart612
Copy link
Contributor

@mumoshu @sledigabel you haven't renamed Docker Hub account yet so .Values.image.repository value is currently broken

@mumoshu
Copy link
Collaborator

mumoshu commented Jun 27, 2021

@jstewart612 Thanks! I'll fix it in #663

mumoshu added a commit that referenced this issue Jun 28, 2021
* Fix image repo name in chart

Ref #644 (comment)
shark314 added a commit to shark314/actions-runner-controller that referenced this issue May 2, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants