ansible-collections / community.kubernetes

Kubernetes Collection for Ansible
https://galaxy.ansible.com/community/kubernetes
GNU General Public License v3.0
265 stars 106 forks source link

make has_plugin detection method more reliable #362

Closed joschi36 closed 3 years ago

joschi36 commented 3 years ago
SUMMARY

This PR makes the plugin detection method more reliable. The issue before was that when you had multiple plugins it wouldn't split at the right point. This method just loops through the list and searches the start of the string for the plugin name.

ISSUE TYPE
COMPONENT NAME

helm

ADDITIONAL INFORMATION

Why the method before was a problem: With multiple plugins with different plugin name lengths installed the detection method failed.

NAME        VERSION         DESCRIPTION                                                                         
diff        3.1.3           Preview helm upgrade changes as a diff                                              
helm-git    0.8.1           Get non-packaged Charts directly from Git.                                          
unittest    0.1.6-rancher1  Unit test for helm chart in YAML with ease to keep your chart functional and robust.

So my line is diff\s\s\s\s\s\t3.1.3[...] and the result variable name with the behavior before was diff\s\s\s\s\s which unfortunately wasn't equal to ´diff´ I hope I could explain it well, If you have an additional question please ask.

/cc @gravesm

codecov[bot] commented 3 years ago

Codecov Report

Merging #362 (72b7772) into main (92e3732) will not change coverage. The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##             main     #362   +/-   ##
=======================================
  Coverage   36.80%   36.80%           
=======================================
  Files           3        3           
  Lines         758      758           
  Branches      148      148           
=======================================
  Hits          279      279           
  Misses        430      430           
  Partials       49       49           

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update 92e3732...72b7772. Read the comment docs.

gravesm commented 3 years ago

@joschi36 thank you for the PR. I think the better solution here would be to just change it to line.split(None, 1). If you searched for a plugin named foo but had one named foobar installed then using startswith("foo") would lead to a false positive.

Also, could you add a changelog fragment? https://docs.ansible.com/ansible/latest/community/development_process.html#creating-a-changelog-fragment

joschi36 commented 3 years ago

Thanks for the feedback. I first also thought on doing that, however I choose to use the more simple variant. But yes you are totally right. I will update the PR with this change and also with the change log fragment.

joschi36 commented 3 years ago

@gravesm I have updated the PR

joschi36 commented 3 years ago

@gravesm I have fixed the double backticks.

github-actions[bot] commented 2 years ago

This repository does not accept pull requests, see the README for details.