feat: add github_enterprise_actions_hosted_runner#3017
feat: add github_enterprise_actions_hosted_runner#3017nico34638 wants to merge 5 commits intointegrations:mainfrom
Conversation
|
👋 Hi! Thank you for this contribution! Just to let you know, our GitHub SDK team does a round of issue and PR reviews twice a week, every Monday and Friday! We have a process in place for prioritizing and responding to your input. Because you are a part of this community please feel free to comment, add to, or pick up any issues/PRs that are labeled with |
|
Hey @nico34638 👋 Thank you very much for your contribution! We're currently working on a major release of the provider and that will possibly delay getting this PR merged. While that's happening, could I bother you for a few changes?
Support for Hosted runners for enterprises was added in v70 of go-github. Our upcoming major release will upgrade to v77. Could you rebase your branch on top of #2891 and use the SDK methods instead of manual HTTP requests? :) |
|
Okay, I made the changes you requested. I rebased on your branch with go github/v77 and changed the description fields. Is that okay with you? I tested it, and with the changes, it still works. |
|
Hey @nico34638 |
|
Hey @deiga, It's done. Do you have any idea when the provider's next release will be? |
github/resource_github_enterprise_actions_hosted_runner_test.go
Outdated
Show resolved
Hide resolved
github/resource_github_enterprise_actions_hosted_runner_test.go
Outdated
Show resolved
Hide resolved
deiga
left a comment
There was a problem hiding this comment.
Please ensure the test structure follows the current pattern
github/resource_github_enterprise_actions_hosted_runner_test.go
Outdated
Show resolved
Hide resolved
github/resource_github_enterprise_actions_hosted_runner_test.go
Outdated
Show resolved
Hide resolved
github/resource_github_enterprise_actions_hosted_runner_test.go
Outdated
Show resolved
Hide resolved
36951d4 to
9ac9b53
Compare
|
I have corrected the comments |
|
Hi @deiga do you any news ? |
stevehipwell
left a comment
There was a problem hiding this comment.
Thanks for the PR @nico34638, I've added some initial review comments.
github/resource_github_enterprise_actions_hosted_runner_test.go
Outdated
Show resolved
Hide resolved
|
Following the last review, I'm taking this opportunity to add two data sources for hosted enterprise runners: |
|
Hi @stevehipwell @deiga Do you have any news ? |
deiga
left a comment
There was a problem hiding this comment.
Please ensure to propagate comments to other places where they would be applicable
06b9a9c to
54d4fbe
Compare
deiga
left a comment
There was a problem hiding this comment.
Please read through your own code and see to it that it's consistent with itself.
I don't know if you use an LLM to generate code or are just in a hurry, but please don't make us comment multiple times on the same issues
|
Thanks for the feedback. I apologize for the inconsistencies. I am really trying my best to keep this PR clean, but it is not that simple. Reviews are spaced about many weeks apart. It is hard to keep track of comments made two months ago. The requirements also keep changing mid-flight. For example, we now need datasources in addition to resources to get merged. On top of that, the codebase is being refactored. I have reviewed the code again and updated a few more details on my end. However, I would really appreciate more specific information on what exactly still needs to change |
Resolves #3016
Before the change?
After the change?
Pull request checklist
Does this introduce a breaking change?
Please see our docs on breaking changes to help!