Check for deprecated/unknown environment variables #1342
Conversation
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #1342 +/- ##
==========================================
+ Coverage 84.88% 84.89% +0.01%
==========================================
Files 100 100
Lines 14520 14547 +27
==========================================
+ Hits 12325 12350 +25
- Misses 1734 1735 +1
- Partials 461 462 +1
Continue to review full report in Codecov by Sentry.
🚀 New features to boost your workflow:
|
| } | ||
|
|
||
| func checkDeprecatedEnvVars() { | ||
| allViperKeys := make(map[string]struct{}) |
There was a problem hiding this comment.
Is the map structure necessary here? From what I see we could use a []string and use slices.Contains to determine if it's in the list.
There was a problem hiding this comment.
I chose the map over slices as it will be the most performant and scalable solution. In slices, the time taken would be leaner O(n), whereas in a map, it will be constant O(1).
| } | ||
| } | ||
|
|
||
| func runDeprecatedEnvVarsTest(t *testing.T, tc deprecatedEnvVarsTest) { |
There was a problem hiding this comment.
Can this instead just be done inside the unit tests instead of splitting it up
There was a problem hiding this comment.
That increases the Cyclomatic complexity of the Test function.
| assert.Equal(t, expected, result) | ||
| } | ||
|
|
||
| type deprecatedEnvVarsTest struct { |
There was a problem hiding this comment.
Can this be moved into the test
There was a problem hiding this comment.
I had to define it outside as the linter gives error if we have more than 5 parameter in a function call.
3 participants
Implemented a function to validate the environment variables passed are part of the new v3 allowed variables if not it should throw a warning message.
Checklist
Before creating a PR, run through this checklist and mark each as complete.
CONTRIBUTINGdocumentmake install-toolsand have attached any dependency changes to this pull requestREADME.md)