Enable all default golangci-lint linters and fix issues related

[Jdubrick]

What does this PR do?

This PR enables the default linters that were disabled in a prior issue and fixes all of the issues presented by the default linters.

Which issue(s) does this PR fix

resolves devfile/api#1302

PR acceptance criteria

• Enable default linters
• All issues reported by make lint are dealt with and make lint passes
• All tests pass in make test

How to test changes / Special notes to the reviewer

You can test these changes by first running make lint to verify there are no more lint errors being reported. You can also run make test to ensure all unit tests also pass with the changes.

[codecov]

Codecov Report

Attention: 4 lines in your changes are missing coverage. Please review.

Comparison is base (719af19) 70.49% compared to head (ef3687d) 73.12%.

Files	Patch %	Lines
pkg/utils/detector.go	78.57%	2 Missing and 1 partial ⚠️
test/apis/utils.go	50.00%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main      #37      +/-   ##
==========================================
+ Coverage   70.49%   73.12%   +2.63%     
==========================================
  Files          11       11              
  Lines        1566     1563       -3     
==========================================
+ Hits         1104     1143      +39     
+ Misses        395      351      -44     
- Partials       67       69       +2     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[thepetk]

In general it looks good to me :) It would be nice if we could add some test cases for the coverage report that is failing.

[Jdubrick]

In general it looks good to me :) It would be nice if we could add some test cases for the coverage report that is failing.

Sounds good. Currently working on trying to increase code coverage for these failing portions, just have to familiarize myself with Go testing

[Jdubrick]

Pushed some changes that increase coverage a little bit for the defer functions and some error cases in detector.go, still doesn't loook like it hits the same coverage as before but it's only different by 0.03% from this link. What are your thoughts @thepetk ?

[thepetk]

Pushed some changes that increase coverage a little bit for the defer functions and some error cases in detector.go, still doesn't loook like it hits the same coverage as before but it's only different by 0.03% from this link. What are your thoughts @thepetk ?

Could you try to add some test cases for the DownloadDevfileTypesFromRegistry too?

[Jdubrick]

@thepetk I added more tests and was able to increase the patch code coverage as a result. From what I was reading online it seemed that a majority of people ignore the err that could arise from resp.Body.Close() as there isn't really anything you can do about it. In this case I can try and remove the err handling for the close, but then we get a potential problem warning from github, see above for reference to that. Once I reintroduced the error handling we dropped just below the 70% cutoff. What do you think should be the course of action?

[Jdubrick]

@thepetk I added more tests and was able to increase the patch code coverage as a result. From what I was reading online it seemed that a majority of people ignore the err that could arise from resp.Body.Close() as there isn't really anything you can do about it. In this case I can try and remove the err handling for the close, but then we get a potential problem warning from github, see above for reference to that. Once I reintroduced the error handling we dropped just below the 70% cutoff. What do you think should be the course of action?

Last commit fixes our issue as I was able to mock the function throwing an error.

[thepetk]

@thepetk I added more tests and was able to increase the patch code coverage as a result. From what I was reading online it seemed that a majority of people ignore the err that could arise from resp.Body.Close() as there isn't really anything you can do about it. In this case I can try and remove the err handling for the close, but then we get a potential problem warning from github, see above for reference to that. Once I reintroduced the error handling we dropped just below the 70% cutoff. What do you think should be the course of action?

Last commit fixes our issue as I was able to mock the function throwing an error.

Very complicated failure from the codecov I have to say. I think the addition we make here with the separate functions will let us keep the coverage higher in the future, so is definitely a good addition.

[thepetk]

/lgtm

[openshift-ci]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: Jdubrick, thepetk

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

• OWNERS [Jdubrick,thepetk]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment