Add support for configuring default squash and merge commit titles and messages#137
Add support for configuring default squash and merge commit titles and messages#137soerenmartius wants to merge 5 commits intomainfrom
Conversation
023855a to
3426b72
Compare
f104e90 to
2c7fb4d
Compare
mariux
left a comment
There was a problem hiding this comment.
- please fix tests
- please fix commit messages
4300586 to
3ac0c9f
Compare
a37c872 to
8104872
Compare
|
@mariux is this good to go? |
mariux
left a comment
There was a problem hiding this comment.
defaults added.. please also adjust docs..
0e7c90a to
5a49d0b
Compare
621847c to
76b9485
Compare
|
@mariux the tests are failing after applying your provided suggestions ... not a priority though https://github.com/mineiros-io/terraform-github-repository/actions/runs/3394710192/jobs/5643646371 |
76b9485 to
90b27c1
Compare
90b27c1 to
bcda3f9
Compare
| squash_merge_commit_title = local.allow_squash_merge ? var.squash_merge_commit_title == null ? try(var.defaults.squash_merge_commit_title, null) : var.squash_merge_commit_title : null | ||
| squash_merge_commit_message = local.allow_squash_merge ? var.squash_merge_commit_title == null ? try(var.defaults.squash_merge_commit_title, null) : var.squash_merge_commit_title : null | ||
| merge_commit_title = local.allow_merge_commit ? var.merge_commit_title == null ? try(var.defaults.merge_commit_title, null) : var.merge_commit_title : null | ||
| merge_commit_message = local.allow_merge_commit ? var.merge_commit_message == null ? try(var.merge_commit_message, null) : var.merge_commit_message : null | ||
|
|
There was a problem hiding this comment.
there is no condition when setting the fields.. we can always set them.
There was a problem hiding this comment.
nope that's not correct, as you can see in this test run, allow_squash_merge xor allow_merge_commit need to be set to true for setting those values otherwise the API returns an error https://github.com/mineiros-io/terraform-github-repository/actions/runs/3394517816/jobs/5643205775#step:4:1092
There was a problem hiding this comment.
the error is based on typos in the variable names.
There was a problem hiding this comment.
Not sure if that's correct, did you see the error
Error: POST https://api.github.com/orgs/***/repos: 422 Repository creation failed. [{Resource:Repository Field: Code:custom Message:Sorry, you need to allow the squash merge strategy in order to set the default squash commit message title or message. (no_squash_merge_strategy)}]
There was a problem hiding this comment.
comitted suggested changes to test... works ;) .... conflicts would need to be resolves and default set to null again... or remove the complex default handling as it's deprectaed now anyway.. ;)
|
Is this ready to be merged? |
|
Is this PR still actually a draft, or once conflicts are resolved, is it ready for approval and merging? Thanks |
|
Would be good to get this over the line. I can't even manually tweak the repository settings as the current release of this module resets the squash settings back to the defaults. |
|
I think this PR might be ready to review? |
|
Hello all! What is the ETA on merging this one? We forked it for the time being just for this PR and it seems to work as expected. |
No description provided.