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

feat: template variables #583

Merged
merged 17 commits into from
Nov 26, 2024
Merged

feat: template variables #583

merged 17 commits into from
Nov 26, 2024

Conversation

brookesargent
Copy link
Contributor

@brookesargent brookesargent commented Nov 22, 2024

Short description of the changes

Support webhook recipient template variables in Terraform

@codecov-commenter
Copy link

codecov-commenter commented Nov 22, 2024

Codecov Report

Attention: Patch coverage is 84.17266% with 22 lines in your changes missing coverage. Please review.

Project coverage is 75.69%. Comparing base (f30002c) to head (250a537).
Report is 2 commits behind head on main.

Files with missing lines Patch % Lines
internal/provider/webhook_recipient_resource.go 84.17% 21 Missing and 1 partial ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #583      +/-   ##
==========================================
+ Coverage   75.36%   75.69%   +0.33%     
==========================================
  Files          87       87              
  Lines        7420     7510      +90     
==========================================
+ Hits         5592     5685      +93     
+ Misses       1471     1469       -2     
+ Partials      357      356       -1     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@brookesargent brookesargent marked this pull request as ready for review November 25, 2024 18:05
@brookesargent brookesargent requested a review from a team as a code owner November 25, 2024 18:05
@jharley jharley added the feature This issue wants to add new functionality. label Nov 25, 2024
Copy link
Contributor Author

@brookesargent brookesargent left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

made some changes to the test file that are a bit annoying to read. I pulled the "errors when they should" t.Run out of the main test and into individual tests that have descriptive names about what exactly is being tested as the number of validations/tests was getting a bit unwieldy

@jharley
Copy link
Collaborator

jharley commented Nov 26, 2024

made some changes to the test file that are a bit annoying to read. I pulled the "errors when they should" t.Run out of the main test and into individual tests that have descriptive names about what exactly is being tested as the number of validations/tests was getting a bit unwieldy

This looks great, thank you!

Copy link
Collaborator

@jharley jharley left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks great, one note about a needed docs addition ✨

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

variable needs to be added as an argument below template

@brookesargent brookesargent merged commit 0266e56 into main Nov 26, 2024
6 checks passed
@brookesargent brookesargent deleted the brooke.template-variables branch November 26, 2024 19:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature This issue wants to add new functionality.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants