mineiros-io / terraform-github-repository

A Terraform module to manage GitHub Repositories. https://github.com/
Apache License 2.0
159 stars 105 forks source link

Add support for use_squash_pr_title_as_default #136

Open robgutsopedra opened 2 years ago

robgutsopedra commented 2 years ago

Github added some months ago this nice option --> https://github.blog/changelog/2022-05-11-default-to-pr-titles-for-squash-merge-commit-messages/

And, as far as I understand, the terraform provider for github already has it. https://github.com/integrations/terraform-provider-github/pull/1263

I don't think I have seen this option in the variables, nor any related issue.

Am I missing something? And, if not, is this something expected to be added?

soerenmartius commented 2 years ago

Github added some months ago this nice option --> https://github.blog/changelog/2022-05-11-default-to-pr-titles-for-squash-merge-commit-messages/

And, as far as I understand, the terraform provider for github already has it. integrations/terraform-provider-github#1263

I don't think I have seen this option in the variables, nor any related issue.

Am I missing something? And, if not, is this something expected to be added?

Hi @robgutsopedra,

Thanks for reaching out and for requesting this! The mentioned commit has been merged in the provider but not been released yet. I will prepare this feature in the next couple of days, but we will need for a new provider release before adding this to the module.

Again, thanks for requesting this!

robgutsopedra commented 2 years ago

Hi @soerenmartius !

Thanks a lot for the super quick reply.

I think from version 4.31 onwards the change is there https://github.com/integrations/terraform-provider-github/releases/tag/v4.31.0

I forked this repo, updated version.tf and added the new variables, and it does seem to work, but documentation on these two fields seems a bit lacking to me. After a couple of tries I got it working perfectly, but I'm not 100% sure about retro compatibility - adding these two fields, even with null value, throws this:

Terraform used the selected providers to generate the following execution plan. Resource actions are indicated with the following symbols:
  ~ update in-place

Terraform will perform the following actions:

  # module.testrep.github_repository.repository will be updated in-place
  ~ resource "github_repository" "repository" {
        id                          = "testrep"
        name                        = "testrep"
      ~ squash_merge_commit_message = "COMMIT_MESSAGES" -> "false"
      ~ squash_merge_commit_title   = "COMMIT_OR_PR_TITLE" -> "false"
        # (31 unchanged attributes hidden)
    }

Plan: 0 to add, 1 to change, 0 to destroy.

So for people specifically wanting to have this feature (in my case, default the squased title to the PR title) works great, otherwise might cause some headaches.

Additionally, if you add some value to squash_merge_commit_title but not to squash_merge_commit_message you might receive some nasty, totally undescriptive errors, like in this situation:

Terraform used the selected providers to generate the following execution plan. Resource actions are indicated with the following symbols:
  ~ update in-place

Terraform will perform the following actions:

  # module.testrep.github_repository.repository will be updated in-place
  ~ resource "github_repository" "repository" {
        id                          = "testrep"
        name                        = "testrep"
      ~ squash_merge_commit_message = "COMMIT_MESSAGES" -> "false"
      ~ squash_merge_commit_title   = "COMMIT_OR_PR_TITLE" -> "PR_TITLE"
        # (31 unchanged attributes hidden)
    }

Plan: 0 to add, 1 to change, 0 to destroy.

Do you want to perform these actions?
  Terraform will perform the actions described above.
  Only 'yes' will be accepted to approve.

  Enter a value: yes

module.testrep.github_repository.repository: Modifying... [id=testrep]
╷
│ Error: PATCH https://api.github.com/repos/ultimateai/testrep: 500  []
│ 
│   with module.testrep.github_repository.repository,
│   on .terraform/modules/testrep/main.tf line 94, in resource "github_repository" "repository":
│   94: resource "github_repository" "repository" {

It doesn't make that much sense that it fails, but I get it. However, I would have explained it a little bit better in the docs.

Hope any of this information is useful to you, and thanks a lot for this module. Is really useful!

soerenmartius commented 2 years ago

Hi @robgutsopedra, thanks for the heads-up! I've implemented the feature in https://github.com/mineiros-io/terraform-github-repository/pull/137 and will get back to you once someone of our team reviewed the PR and when we issued a new release.

Regarding the described issue, unfortunately the GitHub provider is broken in many circumstances :(

robgutsopedra commented 1 year ago

Hey @soerenmartius !

I'd love to have this feature, it would help me quite a lot with some automation I'm working on <3

I see the PR https://github.com/mineiros-io/terraform-github-repository/pull/137 has been stalled for some weeks now, could I lend a hand with something?