-
Notifications
You must be signed in to change notification settings - Fork 2.6k
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: additional template functions #3949
base: master
Are you sure you want to change the base?
Conversation
Welcome @matkam! |
Hi @matkam. Thanks for your PR. I'm waiting for a kubernetes-sigs member to verify that this patch is reasonable to test. If it is, they should reply with Once the patch is verified, the new status will be reflected by the I understand the commands that are listed here. Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
Please submit a CLA per the instructions above. |
Yes 👍 |
@johngmyers we are good to go here 👍 |
source/source.go
Outdated
|
||
func isIPv4String(input string) bool { | ||
netIP := net.ParseIP(input) | ||
return netIP != nil && netIP.To4() != nil |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this function intended to return true
for IPv4-mapped IPv6 addresses?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It just returns true
when the input string is an IPv4 address. It'll return false
if its IPv6 or otherwise invalid IP address.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
how about something like this?
func isIPv4String(input string) bool {
netIP := net.ParseIP(input)
return netIP != nil && netIP.To4() != nil && !strings.Contains(input, ":")
}
Sprig has |
/ok-to-test |
does |
I believe |
sprig looks like a nice package. how about just importing it and using
|
@johngmyers let me know what you think. I've updated the template functions to include Sprig's hermetic text functions, and fixed the |
/retitle feat: additional template functions |
Hello, I would like to see this merged and released. What needs to be done? |
New changes are detected. LGTM label has been removed. |
source/source.go
Outdated
@@ -29,6 +29,7 @@ import ( | |||
"time" | |||
"unicode" | |||
|
|||
sprig "github.com/go-task/slim-sprig" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please no import for a single func.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@szuecs it includes several useful go template functions: https://github.com/go-task/slim-sprig/blob/master/functions.go
Would you rather not include these functions, and only keeping isIPv4
?
The Kubernetes project currently lacks enough contributors to adequately respond to all PRs. This bot triages PRs according to the following rules:
You can:
Please send feedback to sig-contributor-experience at kubernetes/community. /lifecycle stale |
Co-authored-by: Michel Loiseleur <[email protected]>
/remove-lifecycle stale |
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: The full list of commands accepted by this bot can be found here.
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
Description
Adds a few useful text/template functions:
strings.replaceAll
N/A
Checklist