Skip to content

Conversation

jjngx
Copy link
Contributor

@jjngx jjngx commented Sep 27, 2022

Proposed changes

This PR eliminates few dead code paths.

Note that the value of err returned by this func is always nil. This PR is a result of small refactoring that simplifies logic by removing dead code from the configs package.

// GetMapKeyAsStringSlice tries to find and parse a key in the map as string slice splitting it on delimiter.
func GetMapKeyAsStringSlice(m map[string]string, key string, _ apiObject, delimiter string) ([]string, bool, error) {
    if str, exists := m[key]; exists {
	slice := strings.Split(str, delimiter)
	return slice, exists, nil
    }
    return nil, false, nil
}

Refactored func:

// GetMapKeyAsStringSlice tries to find and parse a key in the map as string slice splitting it on delimiter.
func GetMapKeyAsStringSlice(m map[string]string, key string, _ apiObject, delimiter string) ([]string, bool) {
    if str, exists := m[key]; exists {
	slice := strings.Split(str, delimiter)
	return slice, exists
    }
    return nil, false
}

Checklist

Before creating a PR, run through this checklist and mark each as complete.

  • I have read the CONTRIBUTING doc
  • I have added tests that prove my fix is effective or that my feature works
  • I have checked that all unit tests pass after adding my changes
  • I have updated necessary documentation
  • I have rebased my branch onto main
  • I will ensure my PR is targeting the main branch and pulling from my branch from my own fork

@jjngx jjngx requested review from lucacome, ciarams87, vepatel and a team September 27, 2022 08:09
Copy link
Contributor

@shaun-nx shaun-nx left a comment

Choose a reason for hiding this comment

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

👍

@jjngx jjngx requested a review from a team September 27, 2022 10:27
@vepatel vepatel added the chore Pull requests for routine tasks label Sep 27, 2022
@github-actions github-actions bot removed the chore Pull requests for routine tasks label Sep 27, 2022
@ciarams87 ciarams87 added the chore Pull requests for routine tasks label Sep 27, 2022
@codecov-commenter
Copy link

Codecov Report

Merging #3104 (64a88d8) into main (1bc5233) will increase coverage by 0.13%.
The diff coverage is 8.10%.

@@            Coverage Diff             @@
##             main    #3104      +/-   ##
==========================================
+ Coverage   52.25%   52.39%   +0.13%     
==========================================
  Files          58       58              
  Lines       16045    15997      -48     
==========================================
- Hits         8385     8381       -4     
+ Misses       7384     7338      -46     
- Partials      276      278       +2     
Impacted Files Coverage Δ
internal/configs/annotations.go 24.00% <0.00%> (+0.92%) ⬆️
internal/configs/configmaps.go 22.04% <0.00%> (+1.66%) ⬆️
internal/configs/parsing_helpers.go 67.64% <100.00%> (ø)
internal/k8s/configuration.go 95.39% <0.00%> (-0.37%) ⬇️

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@jjngx jjngx merged commit 30df2ca into main Sep 27, 2022
@jjngx jjngx deleted the errcheck branch September 27, 2022 13:32
coolbry95 pushed a commit to coolbry95/kubernetes-ingress that referenced this pull request Nov 18, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
chore Pull requests for routine tasks
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants