Skip to content
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

Add support for use_squash_pr_title_as_default #136

Open
robgutsopedra opened this issue Oct 24, 2022 · 4 comments
Open

Add support for use_squash_pr_title_as_default #136

robgutsopedra opened this issue Oct 24, 2022 · 4 comments
Assignees
Labels
enhancement New feature or request

Comments

@robgutsopedra
Copy link

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?

@soerenmartius
Copy link
Member

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!

@soerenmartius soerenmartius self-assigned this Oct 24, 2022
@soerenmartius soerenmartius added the enhancement New feature or request label Oct 24, 2022
@robgutsopedra
Copy link
Author

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
Copy link
Member

Hi @robgutsopedra, thanks for the heads-up! I've implemented the feature in #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
Copy link
Author

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 #137 has been stalled for some weeks now, could I lend a hand with something?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants