-
Notifications
You must be signed in to change notification settings - Fork 2k
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
Add string comparison functions to Go template executors #4560
Add string comparison functions to Go template executors #4560
Conversation
These string comparison functions come from Go's standard library, so there isn't really a need to test them extensively. Instead, these test cases are mainly to assert that these functions can be correctly used within the template executor.
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.
Thank you for contributing! Could you please keep tests in the same style as in other packages in the project?
- call
t.Parallel()
as a first line in a test - use anonymous struct literals for table tests (as in other test files)
- keep test struct fields not exported
* Call `t.Parallel()` as the first line in each test case. * Use anonymous struct literals for table tests.
@jjngx Thanks for reviewing! I've made the changes for the first 2 points. For the last point, the test struct fields need to be exported for them to be accessed within the templates, else the tests will fail with |
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.
@ianyong thank you again for contributing! 🚀
Codecov Report
@@ Coverage Diff @@
## main #4560 +/- ##
==========================================
+ Coverage 51.97% 51.99% +0.01%
==========================================
Files 59 59
Lines 16962 16960 -2
==========================================
+ Hits 8816 8818 +2
+ Misses 7849 7847 -2
+ Partials 297 295 -2
... and 1 file with indirect coverage changes 📣 Codecov offers a browser extension for seamless coverage viewing on GitHub. Try it in Chrome or Firefox today! |
@ianyong could you please update and push your branch? |
Is there anything I can do to get this PR merged? The failing CI jobs don't seem to be caused by my changes, or at least I see other PRs with the same failed jobs being merged. |
@hanyouqing @ianyong we'll prioritise merging this in coming week, thanks! You can monitor the state from parent issue. |
@ianyong could you please update and push your branch? |
@ianyong could you please update and push your branch? |
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.
👍
Thank you again @ianyong for your contribution! 🎉 |
Proposed changes
Add the following helper functions to the Go template executors to enable string comparisons:
contains
-strings.Contains
hasPrefix
-strings.HasPrefix
hasSuffix
-strings.HasSuffix
toLower
-strings.ToLower
toUpper
-strings.ToUpper
Not very sure why there are two versions of the template executors. I presume
version1
will eventually be phased out in favour ofversion2
given the naming? Either way, I've made the same changes to both versions.Resolves #4555.
Checklist
Before creating a PR, run through this checklist and mark each as complete.