terraform-provider-github
terraform-provider-github copied to clipboard
feat: Split out Pages resource
Resolves #782
Behavior
Before the change?
- When configuring a repository for the first time, one can not specify pages, since a branch needs to exist before pages can be configured.
After the change?
- Decoupling these two concepts allows for more fine control to be used to manage the dependencies, using tools Terraform already gives us.
Other information
- I'm not a Go developer. This code builds, but I'm eager to take more feedback, and to get help from people who know what they are doing more than I do.
Additional info
Pull request checklist
- [ ] Tests for the changes have been added (for bug fixes / features)
- [ ] Docs have been reviewed and added / updated if needed (for bug fixes / features)
- [ ] Added the appropriate label for the given change
Does this introduce a breaking change?
Please see our docs on breaking changes to help!
- [x] Yes (Please add the
Type: Breaking change
label) - [ ] No
If Yes
, what's the impact:
- The
pages
property has been removed from the repository resource - I think this could be added back and deprecated, but it was helpful for me to understand how the code changes if the feature is removed
@nickfloyd What can I do to move this along?
@isaacsanders I think the next best step would be to handle migrations for this breaking change gracefully. We've got some prior art on state migration here. I'm sure there's Hashicorp docs about this somewhere, though I can't seem to find them at the moment.
Are these the docs that you're thinking of @kfcampbell ?
Looks like it, thanks!
@isaacsanders we're planning on cutting a new major version in the near future. Do you have the interest/inclination to continue this work so we can get it merged? If not, perhaps somebody from the community sees value in resuming this feature.
I’d like to make the split happen, but it isn’t something I have a lot of time for right now. The current API is broken, and I think I’ve done enough work in the PR that GitHub could get it over the line.
Would flagging the pages
argument under repository
as deprecated instead of removing it make it easier to get this change out since it wouldn't be a breaking change? Something similar was done with github_repository.default_branch
and github_branch_default
.
@laurenty Ideally we'd do that and at the same time provide a separate resource for managing pages for a repository. I'd like to avoid deprecation without an alternative in place.
@kfcampbell that's exactly what I was suggesting 🙂 This PR adds a new dedicated resource github_repository_pages
, but removes support for GH Pages from github_repository
. I think it would be useful to keep the github_repository
resource as-is, flag the pages
property as deprecated and update the docs to warn user that using both github_repository.pages
and github_repository_pages
will result in unexpected results (just like github_branch_default
does https://registry.terraform.io/providers/integrations/github/latest/docs/resources/branch_default)
Ahh, my mistake! Thank you for correcting me; that's a great course of action.
I’m getting back into coding outside of work, so I should have time to get this in order.
@isaacsanders, if I can help in any way to resume the work on this PR I'd be happy to!
I was trying to resume splitting the GitHub pages from repository resource by adding deprecations to the docs based on the work in this PR, but I don't want to create another stream of changes if this one's still going 🙃