terraform-provider-github icon indicating copy to clipboard operation
terraform-provider-github copied to clipboard

feat: Split out Pages resource

Open isaacsanders opened this issue 1 year ago • 10 comments

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

isaacsanders avatar Mar 23 '23 22:03 isaacsanders

@nickfloyd What can I do to move this along?

isaacsanders avatar Apr 17 '23 13:04 isaacsanders

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

kfcampbell avatar Apr 17 '23 17:04 kfcampbell

Are these the docs that you're thinking of @kfcampbell ?

highb avatar Jun 10 '23 00:06 highb

Looks like it, thanks!

kfcampbell avatar Jun 16 '23 21:06 kfcampbell

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

kfcampbell avatar Oct 17 '23 18:10 kfcampbell

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.

isaacsanders avatar Oct 17 '23 21:10 isaacsanders

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 avatar Jan 24 '24 00:01 laurenty

@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 avatar Feb 05 '24 21:02 kfcampbell

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

laurenty avatar Feb 05 '24 23:02 laurenty

Ahh, my mistake! Thank you for correcting me; that's a great course of action.

kfcampbell avatar Feb 05 '24 23:02 kfcampbell

I’m getting back into coding outside of work, so I should have time to get this in order.

isaacsanders avatar Mar 21 '24 01:03 isaacsanders

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

tpreviero avatar Mar 21 '24 05:03 tpreviero