Add support for configuring default squash and merge commit titles and messages #137
Conversation
soerenmartius
force-pushed
the
soerenmartius/commit-titles
branch
5 times, most recently
from
023855a
to
3426b72
Compare
soerenmartius
force-pushed
the
soerenmartius/commit-titles
branch
from
f104e90
to
2c7fb4d
Compare
soerenmartius
force-pushed
the
soerenmartius/commit-titles
branch
3 times, most recently
from
4300586
to
3ac0c9f
Compare
soerenmartius
changed the title
soerenmartius
force-pushed
the
soerenmartius/commit-titles
branch
6 times, most recently
from
a37c872
to
8104872
Compare
|
@mariux is this good to go? |
soerenmartius
force-pushed
the
soerenmartius/commit-titles
branch
2 times, most recently
from
0e7c90a
to
5a49d0b
Compare
soerenmartius
force-pushed
the
soerenmartius/commit-titles
branch
4 times, most recently
from
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 |
soerenmartius
force-pushed
the
soerenmartius/commit-titles
branch
from
76b9485
to
90b27c1
Compare
soerenmartius
force-pushed
the
soerenmartius/commit-titles
branch
from
90b27c1
to
bcda3f9
Compare
main.tf
Outdated
| 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.