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

refactor!: Replace id input with more general header parameter #11

Merged
merged 3 commits into from
Aug 13, 2024

Conversation

borchero
Copy link
Owner

@borchero borchero commented Jun 2, 2024

Motivation

Fixes #8.

@simonblake-mp, I simply changed the name here. I'm not entirely sure what's the most elegant way to make it configurable. To limit the number of action inputs, having both header (or something like that) and id seems a little overkill so maybe one should use header for both and get rid of id. For now, I don't want to introduce a breaking change though 👀

@borchero borchero self-assigned this Jun 2, 2024
Copy link

github-actions bot commented Jun 2, 2024

📝 Terraform Plan

→ Resource Changes: 1 to create, 0 to update, 1 to re-create, 1 to delete.

✨ Create

local_file.another
+ content              = "Hello terraform-plan-comment!"
+ content_base64sha256 = (known after apply)
+ content_base64sha512 = (known after apply)
+ content_md5          = (known after apply)
+ content_sha1         = (known after apply)
+ content_sha256       = (known after apply)
+ content_sha512       = (known after apply)
+ directory_permission = "0777"
+ file_permission      = "0777"
+ filename             = "../another.txt"
+ id                   = (known after apply)

⚙️ Re-Create

local_file.test[0]
! content              = (sensitive value) # forces replacement
! content_base64sha256 = "ky+KUESlzgvMLeuw3XSV1vkbxOF5V8FyQJOgxHgpBCQ=" -> (known after apply)
! content_base64sha512 = "aJZGbh9jITISv2IG7ZGUUXicGZWE6qx0q+nliS0gv+j+pKarbxV6zulyZVNFRmFbSq37iOY1lZB1Gc2H5dTEBA==" -> (known after apply)
! content_md5          = "fc2c3e6607b6600bc920f94918923a0f" -> (known after apply)
! content_sha1         = "48abe23164c0b8257d86841e4c8f2514083243cd" -> (known after apply)
! content_sha256       = "932f8a5044a5ce0bcc2debb0dd7495d6f91bc4e17957c1724093a0c478290424" -> (known after apply)
! content_sha512       = "6896466e1f63213212bf6206ed919451789c199584eaac74abe9e5892d20bfe8fea4a6ab6f157acee97265534546615b4aadfb88e63595907519cd87e5d4c404" -> (known after apply)
! id                   = "48abe23164c0b8257d86841e4c8f2514083243cd" -> (known after apply)
  # (3 unchanged attributes hidden)

🗑️ Delete

local_file.test[1]
- content              = (sensitive value) -> null
- content_base64sha256 = "ky+KUESlzgvMLeuw3XSV1vkbxOF5V8FyQJOgxHgpBCQ=" -> null
- content_base64sha512 = "aJZGbh9jITISv2IG7ZGUUXicGZWE6qx0q+nliS0gv+j+pKarbxV6zulyZVNFRmFbSq37iOY1lZB1Gc2H5dTEBA==" -> null
- content_md5          = "fc2c3e6607b6600bc920f94918923a0f" -> null
- content_sha1         = "48abe23164c0b8257d86841e4c8f2514083243cd" -> null
- content_sha256       = "932f8a5044a5ce0bcc2debb0dd7495d6f91bc4e17957c1724093a0c478290424" -> null
- content_sha512       = "6896466e1f63213212bf6206ed919451789c199584eaac74abe9e5892d20bfe8fea4a6ab6f157acee97265534546615b4aadfb88e63595907519cd87e5d4c404" -> null
- directory_permission = "0777" -> null
- file_permission      = "0777" -> null
- filename             = "../test.txt" -> null
- id                   = "48abe23164c0b8257d86841e4c8f2514083243cd" -> null

→ because index [1] is out of range for count


Triggered by @borchero, Commit: b31f88ff52e325b2c44afc437cd1d9fb6e27b6be

@simonblake-mp
Copy link

simonblake-mp commented Jun 6, 2024

While that looks grand to me (in terms of resolving my request :-), I'd be inclined to go for a separate header argument that defaults to "## :memo: Terraform Deployment - $id" - then users can override that as they see fit - change the emoji, or alter the heading size markdown, or whatever. Then it wouldn't be a breaking change?

@borchero borchero changed the title refactor: Rename "Terraform Deployment" to "Terraform Plan" refactor!: Replace id input with more general header parameter Aug 13, 2024
@borchero
Copy link
Owner Author

borchero commented Aug 13, 2024

@simonblake-mp sorry this took forever! I decided to go for the breaking change after all since it seems like a more general solution that allows for proper customization without having two parameters that do the same thing :)

@borchero borchero merged commit 66275e7 into main Aug 13, 2024
5 checks passed
@borchero borchero deleted the update-heading branch August 13, 2024 23:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Change the heading of the comment from Deployment to Plan?
2 participants